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

Parse Server crash on FCM response GOAWAY #340

Closed
4 tasks done
mtrezza opened this issue Dec 12, 2024 · 4 comments · Fixed by #341
Closed
4 tasks done

Parse Server crash on FCM response GOAWAY #340

mtrezza opened this issue Dec 12, 2024 · 4 comments · Fixed by #341
Labels
bounty:$20 Bounty applies for fixing this issue (Parse Bounty Program) state:released type:bug

Comments

@mtrezza
Copy link
Member

mtrezza commented Dec 12, 2024

New Issue Checklist

Issue Description

Parse Server crashes when FCM server responds with GOAWAY:

An uncaught exception occurred:
Error while making requests: GOAWAY - server_shutting_down, Error code: 0
Stack Trace:
  at ClientHttp2Session.<anonymous> (/var/app/current/node_modules/firebase-admin/lib/utils/api-request.js:994:23)
  at ClientHttp2Session.emit (node:events:517:28)
  at ClientHttp2Session.emit (node:domain:489:12)
  at Http2Session.onGoawayData (node:internal/http2/core:677:11)

These errors started to appear end of Nov 2024 without a change in the push adapter version, so there has likely been a change in the FCM server behavior. However, even if the firebase-admin SDK doesn't properly handle that response, the adapter should catch this error.

From a quick look into the adapter sending logic it seems that an error should be caught if the promise is rejected:

const allPromises = Promise.all(
slices.map((slice) => sendToDeviceSlice(slice, this.pushType)),
).catch((err) => {
log.error(LOG_PREFIX, `error sending push: ${err}`);
});

However, it seems that the FCM client has a bug and doesn't handle the GOAWAY response properly, i.e. throwing an error instead of rejecting a promise, so that exception propagates all the way to the server. See also firebase/firebase-admin-node#2789.

Steps to reproduce

This issue occurs on network error events, such as GOAWAY or ECONNRESET.

Actual Outcome

Parse Server crashes.

Expected Outcome

Parse Server should handle the error gracefully, log an error and not crash.

Workaround

Add a general exception handler to the Node process.

Environment

Client

  • Parse Server Push Adapter version: 6.7.0

Server

  • Parse Server version: 7.3.0
Copy link

parse-github-assistant bot commented Dec 12, 2024

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

@mtrezza mtrezza added type:bug bounty:$10 Bounty applies for fixing this issue (Parse Bounty Program) labels Dec 12, 2024
@mtrezza mtrezza added bounty:$20 Bounty applies for fixing this issue (Parse Bounty Program) and removed bounty:$10 Bounty applies for fixing this issue (Parse Bounty Program) labels Dec 13, 2024
@siakc
Copy link

siakc commented Dec 14, 2024

Please report your firebase-admin version in /var/app/current/node_modules/firebase-admin/package.json.

@mtrezza
Copy link
Member Author

mtrezza commented Dec 14, 2024

This has been observed with:

"firebase-admin": "12.5.0"
"firebase-admin": "13.0.1"

It seems to me that the issue is specific to the recently added http/2 implementation in firebase-admin SDK.

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty:$20 Bounty applies for fixing this issue (Parse Bounty Program) state:released type:bug
Projects
None yet
3 participants