Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[firebase_auth] Missing Exceptions #3273

Open
feinstein opened this issue Aug 20, 2020 · 16 comments · Fixed by #3317
Open

[firebase_auth] Missing Exceptions #3273

feinstein opened this issue Aug 20, 2020 · 16 comments · Fixed by #3317
Assignees
Labels
platform: all Issues / PRs which are for all platforms. plugin: auth type: documentation Improvements or additions to documentation

Comments

@feinstein
Copy link
Contributor

I am migrating my exceptions to the new ones and on signInWithEmailAndPassword() I can see some changed. These are the old:

  ///  * `ERROR_INVALID_EMAIL` - If the [email] address is malformed.
  ///  * `ERROR_WRONG_PASSWORD` - If the [password] is wrong.
  ///  * `ERROR_USER_NOT_FOUND` - If there is no user corresponding to the given [email] address, or if the user has been deleted.
  ///  * `ERROR_USER_DISABLED` - If the user has been disabled (for example, in the Firebase console)
  ///  * `ERROR_TOO_MANY_REQUESTS` - If there was too many attempts to sign in as this user.
  ///  * `ERROR_OPERATION_NOT_ALLOWED` - Indicates that Email & Password accounts are not enabled.

And these are the new:

  /// A [FirebaseAuthException] maybe thrown with the following error code:
  /// - **invalid-email**:
  ///  - Thrown if the email address is not valid.
  /// - **user-disabled**:
  ///  - Thrown if the user corresponding to the given email has been disabled.
  /// - **user-not-found**:
  ///  - Thrown if there is no user corresponding to the given email.
  /// - **wrong-password**:
  ///  - Thrown if the password is invalid for the given email, or the account
  ///    corresponding to the email does not have a password set.

So ERROR_TOO_MANY_REQUESTS and ERROR_OPERATION_NOT_ALLOWED aren't in the docs anymore.

Just to confirm, is this intentional or are they missing?

@feinstein
Copy link
Contributor Author

I can see the same for many other methods, like reauthenticateWithCredential and updatePassword.

@Ehesp
Copy link
Member

Ehesp commented Aug 21, 2020

Hmm, those ones are not method specific and can happen across them all - maybe worth adding these to the FirebaseAuthException class.

@Salakar Salakar changed the title [firebase_aut] Missing Exceptions [firebase_auth] Missing Exceptions Aug 21, 2020
@Salakar
Copy link
Member

Salakar commented Aug 21, 2020

As @Ehesp mentioned these 2 errors specifically can happen on any auth call. We should definitely still list them somewhere, perhaps the FirebaseAuthException class as mentioned.

Would you mind sending over a PR @feinstein? This would be helpful, thanks

@feinstein
Copy link
Contributor Author

I don't mind, but I think this could be tricky, people will only see the current list of exceptions and won't look the FirebaseAuthException docs to see there's more. Thoughts? Should we place a warning on the docs of very method? Should it be on the website docs?

@Salakar
Copy link
Member

Salakar commented Aug 21, 2020

I think that's fair, could add in both places then?

@feinstein
Copy link
Contributor Author

feinstein commented Aug 21, 2020

I don't think this is just about 2 places, is it? How many exception docs were changed? Is this only a problem for Firebase Auth?

@KristianBalaj
Copy link

#3402 check out this PR. It could solve some exception codes struggles 👋

@feinstein
Copy link
Contributor Author

feinstein commented Sep 1, 2020

As I update the APIs my App is using, I am also finding other exceptions mismatches:

  • reauthenticateWithCredential():

    • Old Exceptions missing:

      • ERROR_USER_DISABLED
      • ERROR_OPERATION_NOT_ALLOWED
    • New Exceptions not matching old ones:

      • user-mismatch
      • invalid-email
      • invalid-verification-code
      • invalid-verification-id
  • updatePassword():

    • Old Exceptions missing:
      • ERROR_USER_DISABLED
      • ERROR_USER_NOT_FOUND
  • signInWithEmailAndPassword():

    • Old Exceptions missing:
      • ERROR_TOO_MANY_REQUESTS
      • ERROR_OPERATION_NOT_ALLOWED
  • signInWithCredential():

    • Old Exceptions missing:

      • ERROR_INVALID_ACTION_CODE
    • New Exceptions not matching old ones:

      • user-not-found
      • wrong-password
      • invalid-verification-code
      • invalid-verification-id
  • sendPasswordResetEmail():

    • New Exceptions not documented:
      • invalid-email
      • user-not-found
  • updateProfile():

    • Old Exceptions missing:
      • ERROR_USER_DISABLED
      • ERROR_USER_NOT_FOUND
  • User.updateEmail():

    • Old Exceptions missing:
      • ERROR_USER_DISABLED
      • ERROR_USER_NOT_FOUND
      • ERROR_OPERATION_NOT_ALLOWED

So, which of these should be included into FirebaseAuthException and which ones are missing from the function docs? What are their new codes?

You only asked me to include the 2 that I found missing at first (ERROR_TOO_MANY_REQUESTS and ERROR_OPERATION_NOT_ALLOWED), but now I also found ERROR_USER_DISABLED and ERROR_USER_NOT_FOUND, which makes me ask: are there other FirebaseAuthException I am missing, from other functions that I don't use in my project, that also need to be documented?

@KristianBalaj
Copy link

@feinstein check out the #3402 PR. The AuthExceptionStatusCode enum contains all the possible status codes I've found in the tests. I've also documented them in the file 🙌

@feinstein
Copy link
Contributor Author

@KristianBalaj I saw your PR, but it doesn't answer the questions:

  1. Which old Exceptions missing from the docs, are from the framework, which are from the functions and which (if any) are just missing? This is important so we know were to document them.

  2. What are the new codes of the old missing Exceptions?

@KristianBalaj
Copy link

@feinstein from the PR. There are all the codes (found in the tests). That's the response to 2.

@feinstein
Copy link
Contributor Author

feinstein commented Sep 1, 2020

@KristianBalaj thanks, I can see the old Exceptions missing with similar names in that file.

But still @Ehesp we need to better document this so our App can better handle alerts for the user, explaining what went wrong and how to fix it or contact support.

We need a list of all the exceptions that can be thrown at any time, we also need to reference that list at any function that could have them thrown.

@Ehesp
Copy link
Member

Ehesp commented Sep 2, 2020

@feinstein Put my thoughts in the PR: #3402 (comment)

@feinstein
Copy link
Contributor Author

There's a new exception invalid-credential since email enumeration protection was activated, but the signInWithEmailAndPassword docs doesn't include it in the exceptions list.

@Lyokone Lyokone added the platform: all Issues / PRs which are for all platforms. label Mar 19, 2024
@tushar0518
Copy link

@Salakar @Lyokone @Ehesp @feinstein
is there any update regarding this issue of the "sendPasswordResetEmail" method not catching exceptions? as mentioned above by @darshankawar in Flutter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: all Issues / PRs which are for all platforms. plugin: auth type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants