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_web] handle exception as documented in platform interface. #1559

Closed
GregorySech opened this issue Dec 5, 2019 · 15 comments
Closed
Labels
blocked: customer-response Waiting for customer response, e.g. more information was requested. platform: web Issues / PRs which are specifically for web. plugin: auth resolution: needs-repro This issue could not be reproduced or needs an up to date reproduction on latest FlutterFire plugin. type: bug Something isn't working

Comments

@GregorySech
Copy link
Contributor

GregorySech commented Dec 5, 2019

Describe the bug
Exceptions in the web implementation are not catchable using the platform interface.

To Reproduce
Steps to reproduce the behavior:

  1. Create an empty flutter project using master
  2. Setup firebase_auth and firebase_auth_web following the web package README. (I didn't activate email auth provider to make sure of getting an API error).
  3. Place a try catch block to catch a possible error using AuthException
  4. Try to cause the expected error. I tried to authenticate with email and password without activating the email provider to cause ERROR_OPERATION_NOT_ALLOWED.

Expected behavior
I expect the try catch to trigger but it gets ignored. The error is instead reported in the development console as "uncaught in Promise".

Additional context
I'm working on a fix. I will create an "exception mapper" from package:firebase's FirebaseError to AuthException using this reference.

I'm working on the mapping here https://docs.google.com/spreadsheets/d/1VrmcLK3-KF5_DK732XDA0G64wxgiP4W3KvwxeP6Y_DA/edit?usp=sharing

@GregorySech GregorySech added the type: bug Something isn't working label Dec 5, 2019
@collinjackson
Copy link
Contributor

/cc @hterkelsen

@GregorySech
Copy link
Contributor Author

GregorySech commented Dec 5, 2019

I'm kind of stuck, I need someone familiar with firebasejs to understand if some errors are missing from the mapping but not documented in the reference docs. Also I'm not sure what exactly is auth/unauthorized-domain about.

@collinjackson
Copy link
Contributor

@GregorySech If you have specific questions feel free to post them here, or if you want to put up a PR with your initial attempt we can review it.

@GregorySech
Copy link
Contributor Author

GregorySech commented Dec 6, 2019

I've not written code so far, I'm working on getting a decent mapping first on the linked sheet. Once that's done I'll start the PR.

My first question would be to @slightfoot about ERROR_UNAUTHORIZED_DOMAIN. In his sheet that error is linked to the same description of auth/unauthorized-continue-uri per js reference. The problem is that auth/unauthorized-domain exists but has a different meaning per js reference. Is that an error or in the mobile SDKs unauthorized domain has a different meaning?

After that I have a lot of errors that seem to not have correspondence to the js API.
Such as:
ERROR_USER_MISMATCH
ERROR_INVALID_MESSAGE_PAYLOAD
ERROR_INVALID_SENDER
ERROR_INVALID_RECIPIENT_EMAIL
ERROR_MISSING_EMAIL
ERROR_MISSING_PASSWORD
MISSING_APP_CREDENTIAL
INVALID_APP_CREDENTIAL
ERROR_SESSION_EXPIRED*
ERROR_MISSING_APP_TOKEN
ERROR_NOTIFICATION_NOT_FORWARDED
ERROR_APP_NOT_VERIFIED
ERROR_WEB_CONTEXT_ALREADY_PRESENTED
ERROR_WEB_CONTEXT_CANCELLED
ERROR_APP_VERIFICATION_FAILED
ERROR_INVALID_CLIENT_ID
ERROR_WEB_NETWORK_REQUEST_FAILED
ERROR_WEB_INTERNAL_ERROR
ERROR_KEYCHAIN_ERROR
ERROR_INTERNAL_ERROR
ERROR_MALFORMED_JWT

To find if those are actually missing I should either find someone that can confirm them as missing or go through the js SDK source. If someone already encountered them I would be thankful otherwise I'll start going through the source this evening.

EDIT: I've found the error sources and I'm starting to adding the missing mappings. In the evening I will remove what I've mapped.

@slightfoot
Copy link
Contributor

@GregorySech I built the original spreadsheet from the source code of both SDKs and the docs for Firebase Auth for both of them too. It could be a mistake in the docs?

@GregorySech
Copy link
Contributor Author

GregorySech commented Dec 6, 2019

Yeah, it's probably a mistake in the docs. I guess i'll just make a new error value for unauthorized-continue-uri. How do you guys feel about having a list of the possible error values in one of the packages? If you like the idea should I put it in the "master" package (firebase_auth) or in the platform interface?

edit: I'm thinking that those errors have actually been collapsed in the mobile sdks as unauthorized-domain is a generalization that contains unauthorized-continue-uri.

@GregorySech
Copy link
Contributor Author

I'm done with implementing the mapper here. I need to add tests and update documentation.
I need some direction on both.
About tests: should I just make the mapping function visible inside firebase_auth_web and exercise it or try something more integrated with the actual calls?
On documentation I will look through the reference for missing errors in the firebase_auth doc-blocks.

@harryterkelsen
Copy link

Thanks for doing this! Your approach LGTM for the most part. You can make the mapping function @visibleForTesting so it won't be part of the public API.

@yumin-chen
Copy link

yumin-chen commented Dec 22, 2019

Thank you for implementing this exception mapping to handle FirebaseError from the web.

I saw that you are mapping all FirebaseError to AuthException in your implementation. However, FirebaseAuth seems to actually throw a PlatformException rather than AuthException. See previous dicussion on #725 and flutter/plugins#775, both threads are referencing to PlatformException. Shouldn't this be mapped to PlatformException instead?

In my own experience, if you try and catch AuthException, it won't be caught. Even though the error code is part of the auth module.

Unhandled Exception: PlatformException(ERROR_INVALID_CREDENTIAL, Unable to verify the ID Token signature.

Above example is when signInWithCredential throws a PlatformException (code: auth/invalid-credential)

PlatformException is from the flutter/services.dart package:

import 'package:flutter/services.dart' show PlatformException;

@GregorySech
Copy link
Contributor Author

Thanks @chen-yumin totally missed that.
Mapping everything to PlatformException is easy this afternoon I'll change the mapping, realign my fork and open the PR.
Wondering what AuthException is for tho.

@illia-romanenko
Copy link

Hi, @GregorySech

any updates on this? Do you need any help with finishing this PR?

@GregorySech
Copy link
Contributor Author

Hi, @GregorySech

any updates on this? Do you need any help with finishing this PR?

On my end the merge conflicts are solved. I'll ask to get the PR reviewed again.

@wer-mathurin
Copy link
Contributor

Sonner than later will be great. Working on a customer project and this can simplify a lot the errors handling since we need to support both web and native.

@harisijazwarraich
Copy link

harisijazwarraich commented Aug 18, 2020

Hello, the exceptions can be handled using, FirebaseAuthException class.

void loginUser(String email, String password) async { try { await _auth.signInWithEmailAndPassword(email: email, password: password); } on FirebaseAuthException catch (e) { Get.snackbar('Error logging in!', e.message, snackPosition: SnackPosition.BOTTOM); } catch (e) { // For all other exceptions! }

I am using GetX for state management here, Get.snackbar() is used to show a snackbar showing the error, you can use your own logic to handle the error, e.g show a alert dialog e.t.c
Cheers <3

@Salakar Salakar added blocked: customer-response Waiting for customer response, e.g. more information was requested. resolution: needs-repro This issue could not be reproduced or needs an up to date reproduction on latest FlutterFire plugin. labels Aug 25, 2020
@Salakar
Copy link
Member

Salakar commented Aug 25, 2020

Hey 👋

Our rework of the firebase_auth plugin as part of the FlutterFire roadmap was published over a week ago with a ton of fixes and new features. Please could you try the new version and see if this is still an issue for you? If it is then please submit a new up to date GitHub issue. There was lot of work done to improve error handling: https://firebase.flutter.dev/docs/auth/error-handling

For help migrating to the new plugins please see the new migration guide: https://firebase.flutter.dev/docs/migration

@Salakar Salakar closed this as completed Aug 25, 2020
@firebase firebase locked and limited conversation to collaborators Sep 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked: customer-response Waiting for customer response, e.g. more information was requested. platform: web Issues / PRs which are specifically for web. plugin: auth resolution: needs-repro This issue could not be reproduced or needs an up to date reproduction on latest FlutterFire plugin. type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants