LaunchDarkly node-server-sdk is crashing node process due to unhandled promise

[jsk95]

Describe the bug
Currently, if incomingmessage errors due to intermittent network issues such as ECONNRESET, as the promise in the constructor in NodeResponse is unhandled, this bubbles up as unhandledRejection, crashing the process.

js-core/packages/sdk/server-node/src/platform/NodeResponse.ts

Line 31 in 6349b98

reject(err);

This is happening quite frequently, affecting our production environment.

To reproduce
This should be reproducible with a sample application that talks to the server but terminates the connection. I did not spend too much effort in getting a repro ready as the code fault and error location is clear.

Expected behavior
Rejection should not be unhandled and if request fails, LaunchDarkly SDK should serve the default value gracefully.

Logs
Note: promise has been monkey patched to get the stack trace of the promise init.

unhandledRejection

Error: aborted
    at connResetException (node:internal/errors:720:14)
    at TLSSocket.socketCloseListener (node:_http_client:461:19)
    at TLSSocket.emit (node:events:529:35)
    at TLSSocket.emit (node:domain:489:12)
    at node:net:350:12
    at TCP.done (node:_tls_wrap:657:7)

call site stack trace

at new MonkeyPromise (/var/service/out/lib/app.js:13:19)
    at new NodeResponse (/var/service/node_modules/@launchdarkly/node-server-sdk/dist/src/platform/NodeResponse.js:12:24)
    at ClientRequest.<anonymous> (/var/service/node_modules/@launchdarkly/node-server-sdk/dist/src/platform/NodeRequests.js:86:33)
    at Object.onceWrapper (node:events:632:26)
    at ClientRequest.emit (node:events:517:28)
    at ClientRequest.emit (node:domain:489:12)
    at HTTPParser.parserOnIncomingClient (node:_http_client:700:27)
    at HTTPParser.parserOnHeadersComplete (node:_http_common:119:17)
    at TLSSocket.socketOnData (node:_http_client:541:22)
    at TLSSocket.emit (node:events:517:28)

SDK version
@launchdarkly/node-server-sdk@8.2.4

Language version, developer tools
node v18

OS/platform
ubuntu:jammy@sha256:6042500cf4b44023ea1894effe7890666b0c5c7871ed83a97c36c76ae560bb9b

[kinyoklion]

Hello @jsk95,

Thank you for the issue.

In order to narrow down the conditions for this, can you share a snippet of your SDK configuration.

I am primarily interested if you are disabling streaming. If streaming is enabled, then the only pathway that is making http requests, that take this path, is event delivery.

In the case of event delivery there is not a response body, so we are not awaiting this promise. (When polling we do await the promise, and there are not any intermediate awaits that should allow this situation.) It does await the request, which is

So, most probably, this is happening, and I can re-arrange some handling to prevent that.

If it is happening from a poll request, then that is far more problematic as there isn't a path where the response isn't awaited.

Thanks,
Ryan

Filed internally as 232944

[jsk95]

Thanks @kinyoklion - we basically use the default setting for LD SDK with just diagnosticOptOut: true enabled, so we should be using streaming. Like you said there are not many async events that could cause the error between the time the promise gets created and .text() gets awaited on even for polling but it would probably be best not left up to chance.

[jsk95]

I see that you have a potential fix up already - we would also appreciate an ETA for the fix as well, if possible. This does put our production systems at risk.

[kinyoklion]

@jsk95 I will have a release out for 9.x today.

I have been working on validation and have discovered that this issue only is not reproducible on node 14, but does seem to be reproducible on 16+. Now that I can reliably reproduce the issue I can validate the fix.

If you need an 8.x release, then I will most likely not have that today. That said 9.x just adds migration support and deprecates, but doesn't remove, LDUser, so it is mostly a drop-in replacement.

Thanks,
Ryan

[kinyoklion]

9.1.1 has been released with this fix.

[kinyoklion]

This has been replaced in 8.2.5.

[jsk95]

Thanks @kinyoklion - we've replaced it with v9 latest and we have not seen it re-occur so it is looking good!