Skip to content
Advertisement

Code runs after catch statement catches and error and returns in react native firebase

I am having issue whenever I catch an error and return from a function, by code after the catch block still runs. Here is my two functions that I use:

    usernameTaken: async (username) => {
        const user = await firebase.firestore().collection("uniqueUsers").doc(username).get();
        if (user.exists) {
            alert("Username is taken. Try again with another username.");
            throw new Error('Username is taken. Try again with another username.');
        }
    },
    changeUsername: async (currentUsername, newUsername) => {
      try {
          var user = Firebase.getCurrentUser();
          Firebase.usernameTaken(newUsername);
      } catch (err) {
          alert(err.message);
          return;
      }
      await db.collection('uniqueUsers').doc(currentUsername).delete();
      await db.collection("users").doc(user.uid).update({username: newUsername});
      await db.collection("uniqueUsers").doc(newUsername).set({username: newUsername});
      alert("Congratulations! You have successfully updated your username.");
    }

I would greatly appreciate any help for this problem, as I have been struggling with it for over 2 days now and can’t seem to find a solution.

Advertisement

Answer

In your original code, the usernameTaken() promise is floating, because you didn’t use await. Because it was floating, your catch() handler will never catch it’s error.

changeUsername: async (currentUsername, newUsername) => {
  try {
      const user = Firebase.getCurrentUser();
      /* here -> */ await Firebase.usernameTaken(newUsername);
  } catch (err) {
      alert(err.message);
      return;
  }
  /* ... other stuff ... */
}

Additional Points

usernameTaken should return a boolean

You should change usernameTaken to return a boolean. This is arguably better rather than using alert() (which blocks execution of your code) or throwing an error.

usernameTaken: async (username) => {
  const usernameDoc = await firebase.firestore().collection("uniqueUsers").doc(username).get();
  return usernameDoc.exists; // return a boolean whether the doc exists
}

Securely claim and release usernames

Based on your current code, you have no protections for someone coming along and just deleting any usernames in your database or claiming a username that was taken between the time you last checked it’s availability and when you call set() for the new username. You should secure your database so that a user can only write to a username they own.

Add the owner’s ID to the document:

"/uniqueUsers/{username}": {
  username: "username",
  uid: "someUserId"
}

This then allows you to lock edits/deletions to the user who owns that username.

service cloud.firestore {
  match /databases/{database}/documents {
    
    match /uniqueUsers/{username} {
      // new docs must have { username: username, uid: currentUser.uid }
      allow create: if request.auth != null
                    && request.resource.data.username == username
                    && request.resource.data.uid == request.auth.uid
                    && request.resource.data.keys().hasOnly(["uid", "username"]);

      // any logged in user can get this doc
      allow read: if request.auth != null;

      // only the linked user can delete this doc
      allow delete: if request.auth != null
                    && request.auth.uid == resource.data.uid;

      // only the linked user can edit this doc, as long as username and uid are the same
      allow update: if request.auth != null
                    && request.auth.uid == resource.data.uid
                    && request.resource.data.diff(resource.data).unchangedKeys().hasAll(["uid", "username"]) // make sure username and uid are unchanged
                    && request.resource.data.diff(resource.data).changedKeys().size() == 0; // make sure no other data is added
    }
  }
}

Atomically update your database

You are modifying your database in a way that could corrupt it. You could delete the old username, then fail to update your current username which would mean that you never link your new username. To fix this, you should use a batched write to apply all these changes together. If any one were to fail, nothing is changed.

await db.collection("uniqueUsers").doc(currentUsername).delete();
await db.collection("users").doc(user.uid).update({username: newUsername});
await db.collection("uniqueUsers").doc(newUsername).set({username: newUsername});

becomes

const db = firebase.firestore();
const batch = db.batch();

batch.delete(db.collection("uniqueUsers").doc(currentUsername));
batch.update(db.collection("users").doc(user.uid), { username: newUsername });
batch.set(db.collection("uniqueUsers").doc(newUsername), { username: newUsername });

await batch.commit();

Usernames should be case-insensitive

Your current usernames are case-sensitive which is not recommended if you expect your users to type/write out their profile’s URL. Consider how "example.com/MYUSERNAME", "example.com/myUsername" and "example.com/myusername" would all be different users. If someone scribbled out their username on a piece of paper, you’d want all of those to go to the same user’s profile.

usernameTaken: async (username) => {
  const usernameDoc = await firebase.firestore().collection("uniqueUsers").doc(username.toLowerCase()).get();
  return usernameDoc.exists; // return a boolean whether the doc exists
},
changeUsername: async (currentUsername, newUsername) => {
  const lowerCurrentUsername = currentUsername.toLowerCase();
  const lowerNewUsername = newUsername.toLowerCase();

  /* ... */

  return lowerNewUsername; // return the new username to show success
}

The result

Combining this all together, gives:

usernameTaken: async (username) => {
  const usernameDoc = await firebase.firestore().collection("uniqueUsers").doc(username).get();
  return usernameDoc.exists; // return a boolean
},
changeUsername: async (currentUsername, newUsername) => {
  const user = Firebase.getCurrentUser();
  if (user === null) {
    throw new Error("You must be signed in first!");
  }

  const taken = await Firebase.usernameTaken(newUsername);
  if (taken) {
    throw new Error("Sorry, that username is taken.");
  }

  const lowerCurrentUsername = currentUsername.toLowerCase();
  const lowerNewUsername = newUsername.toLowerCase();
  const db = firebase.firestore();
  const batch = db.batch();
  
  batch.delete(db.collection("uniqueUsers").doc(lowerCurrentUsername));
  batch.update(db.collection("users").doc(user.uid), {
    username: lowerNewUsername
  });
  batch.set(db.collection("uniqueUsers").doc(lowerNewUsername), {
    username: lowerNewUsername,
    uid: user.uid
  });

  await batch.commit();

  return lowerNewUsername;
}
// elsewhere in your code
changeUsername("olduser", "newuser")
  .then(
    (username) => {
      alert("Your username was successfully changed to @" + username + "!");
    },
    (error) => {
      console.error(error);
      alert("We couldn't update your username!");
    }
  );

Note: If you are using all of the above recommendations (like the security rules), one of the expected ways batch.commit() will fail is if someone takes the username before the current user. If you get a permissions error, assume that someone took the username before you.

Advertisement