-
Notifications
You must be signed in to change notification settings - Fork 379
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
fix(fcm): Wrap HTTP/2 session errors in promise #2868
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jonathan! Added one comment
// Start session listeners | ||
http2SessionHandler.invoke().catch((error) => { | ||
const pendingBatchResponse = | ||
sendResponsePromise ? sendResponsePromise.then(this.parseSendResponses) : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking out loud, if a session error occurs before any of the requests have gone through would pendingBatchResponse
be undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this would be undefined.
RELEASE NOTE: Fixed a issue which caused session errors not to be thrown.
FirebaseMessagingSessionError
pendingBatchResponse
field to newFirebaseMessagingSessionError
to allow access to the status of messages which were in flight at error time.