Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Unhandled promise rejection when flushing events #85

Closed
jbunton-atlassian opened this issue Mar 27, 2018 · 4 comments
Closed

Unhandled promise rejection when flushing events #85

jbunton-atlassian opened this issue Mar 27, 2018 · 4 comments
Labels

Comments

@jbunton-atlassian
Copy link

client.flush returns a promise but nothing is attached to handle rejections. This is deprecated in Node and in the future will cause the process to exit in a similar manner to an unhandled exception.

In our production services we have already configured unhandled rejections to cause a process exit. I noticed this bug because timeouts in the LaunchDarkly client are causing our processes to die when there is a network timeout.

I've created a demo repo if you want to try this out:
https://github.com/jbunton-atlassian/launchdarkly-bug-promise-rejection

It hardcodes a black hole IP address to simulate a timeout. Run it with the SDK_KEY and FEATURE_KEY environment variables. After a few seconds the automatic flush will trigger and you'll see the promise rejection warning.

Thanks :)

@apucacao
Copy link
Contributor

Hi @jbunton-atlassian ,

Thanks for reporting this. And sorry for the inconvenience. We're going to put a fix out shortly to address this.

I'll keep you posted here.

@jbunton-atlassian
Copy link
Author

Awesome! Thanks for the quick fix :D

@eli-darkly
Copy link
Contributor

The 4.0.3 release should fix this, and I tested it in your sample code, but please feel free to reopen the issue if something still doesn't look right.

eli-darkly added a commit that referenced this issue Jun 26, 2018
mark waitUntilReady as deprecated
@eli-darkly
Copy link
Contributor

Unfortunately, we just found out that this bug fix was partially undone by a regression in version 5.0.0, which once again made it possible for event flush errors to trigger an unhandled promise rejection. This has been re-fixed in 5.4.2, and there is now better test coverage to prevent another such regression.

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

No branches or pull requests

3 participants