Fix calls awaiting forever on firebase scripts failing to load on web#204
Fix calls awaiting forever on firebase scripts failing to load on web#204TomVHPresto wants to merge 3 commits intocapacitor-community:masterfrom
Conversation
TomVHPresto
changed the title
|
Any update? |
| await this.loadScript( | ||
| firebaseAnalyticsScript.key, | ||
| firebaseAnalyticsScript.src | ||
| resolve( |
There was a problem hiding this comment.
Similar reasoning here, using await will throw an exception on error which causes an unhandled promise rejection - instead resolving it using this promise propagates any error by rejecting the promise.
There was a problem hiding this comment.
I think I was wrong about this actually and only the other change is necessary. Let me do some testing
There was a problem hiding this comment.
It seems it won't work without the change in my testing. Using async / await inside a new promise constructor is an anti-pattern because if it throws inside an async callback, the error isn't propagated correctly.
| } | ||
| } catch (error) { | ||
| throw error; | ||
| this.readyReject(error); |
There was a problem hiding this comment.
The error being thrown here means that the ready promise is never rejected, so if you do await FirebaseAnalytics.initialize() you can't catch the error using a try / catch, and the call awaits forever. This was causing an issue for me because I work on a web app that can be used offline, so I need to be able to handle his gracefully.
robingenz
left a comment
robingenz
left a comment
There was a problem hiding this comment.
Please run npm run changeset and generate a changeset file.
This script doesn't seem to exist |
|
Any update? |
|
@TomVHPresto Sorry, I mixed up the repo with https://github.com/capawesome-team/capacitor-firebase. Someone else from the community should take a look at this PR. I'm not that active in this repo. |
|
Well thanks anyway 😂 |
2 participants
On web, currently if the firebase scripts fail to load, throws an error from inside the Promise callback, which causes an unhandled promise rejection. This means that the ready promise is never resolved or rejected, and function calls like initializeFirebase will await forever.
This change means that the ready promise will be rejected and the errors will be propagated upwards correctly.