flutterfire icon indicating copy to clipboard operation
flutterfire copied to clipboard

[firebase_auth] Missing Exceptions

Open feinstein opened this issue 5 years ago • 16 comments

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 avatar Aug 20 '20 23:08 feinstein

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

feinstein avatar Aug 20 '20 23:08 feinstein

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

Ehesp avatar Aug 21 '20 07:08 Ehesp

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

Salakar avatar Aug 21 '20 11:08 Salakar

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?

feinstein avatar Aug 21 '20 12:08 feinstein

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

Salakar avatar Aug 21 '20 12:08 Salakar

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?

feinstein avatar Aug 21 '20 12:08 feinstein

Lets add it here: https://firebase.flutter.dev/docs/auth/error-handling & here: https://pub.dev/documentation/firebase_auth_platform_interface/latest/firebase_auth_platform_interface/FirebaseAuthException-class.html (below the class description).

Ehesp avatar Aug 21 '20 13:08 Ehesp

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

KristianBalaj avatar Aug 30 '20 12:08 KristianBalaj

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?

feinstein avatar Sep 01 '20 01:09 feinstein

@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 🙌

KristianBalaj avatar Sep 01 '20 06:09 KristianBalaj

@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?

feinstein avatar Sep 01 '20 17:09 feinstein

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

KristianBalaj avatar Sep 01 '20 17:09 KristianBalaj

@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.

feinstein avatar Sep 01 '20 17:09 feinstein

@feinstein Put my thoughts in the PR: https://github.com/FirebaseExtended/flutterfire/pull/3402#issuecomment-685402151

Ehesp avatar Sep 02 '20 07:09 Ehesp

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.

feinstein avatar Feb 21 '24 11:02 feinstein

@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.

tushar0518 avatar Aug 26 '24 12:08 tushar0518