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

Catch to prevent push-receiver crash on disconnect #1540

Open
wants to merge 1 commit into
base: beta
Choose a base branch
from

Conversation

tsightler
Copy link
Collaborator

This patch addresses an unhandled promise rejection that occurs in push-receiver 4.3.0. This version of push-receiver introduced a new whenReady promise that consumers can use to know when the initial connection is ready. However, it seems to have a bug when the socket connection fails as the code rejects the promise but has no catch for this rejection, thus creating an unhandled promise which leads to a process crash on modern NodeJS >15 which exit on unhandled promises by default.

It's possible to attach a catch handler outside of the library during first use, but the issue is that a new promise is created on every connection attempt, so, while that will catch the initial failure, subsequent retries are new promises without a catch.

This patch takes a different approach, instead of attaching the catch handler during creating, it instead listens for "ON_DISCONNECT" event, which happens just before the promise rejection, and immediately attaches a catch handler to the promise to prevent the unhandled rejection error. It's a big of a hack, but seems to work reliably in testing until we can get upstream to fix the behavior (issue is already opened).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant