firebase / flutterfire

🔥 A collection of Firebase plugins for Flutter apps.
https://firebase.google.com/docs/flutter/setup
BSD 3-Clause "New" or "Revised" License
8.67k stars 3.97k forks source link

[firebase_auth] Missing Exceptions #3273

Open feinstein opened 4 years ago

feinstein commented 4 years ago

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 commented 4 years ago

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

Ehesp commented 4 years ago

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

Salakar commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

feinstein commented 4 years ago

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?

Ehesp commented 4 years ago

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

KristianBalaj commented 4 years ago

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

feinstein commented 4 years ago

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

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

feinstein commented 4 years ago

@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 commented 4 years ago

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

feinstein commented 8 months ago

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.

tushar0518 commented 1 month ago

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