-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Comments
/cc @hterkelsen |
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. |
@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. |
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. 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. |
@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? |
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. |
I'm done with implementing the mapper here. I need to add tests and update documentation. |
Thanks for doing this! Your approach LGTM for the most part. You can make the mapping function |
Thank you for implementing this exception mapping to handle I saw that you are mapping all In my own experience, if you try and catch
Above example is when
|
Thanks @chen-yumin totally missed that. |
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. |
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. |
Hello, the exceptions can be handled using, FirebaseAuthException class.
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 |
Hey 👋 Our rework of the For help migrating to the new plugins please see the new migration guide: https://firebase.flutter.dev/docs/migration |
Describe the bug
Exceptions in the web implementation are not catchable using the platform interface.
To Reproduce
Steps to reproduce the behavior:
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
The text was updated successfully, but these errors were encountered: