Open homer0 opened 1 year ago
Hello @homer0,
This says SDK version latest, but the latest SDK is not located in this repository.
The latest SDK is here: https://github.com/launchdarkly/js-core/tree/main/packages/sdk/server-node
And is published under @launchdarkly/node-server-sdk
.
That said it does appear the latest version would operate the same way, and will need fixed.
The problem now residing in this method: https://github.com/launchdarkly/js-core/blob/63697e15395dfb861e67528818799e90375ce80e/packages/sdk/server-node/src/platform/NodeRequests.ts#L109
What is the external behavior that is causing you a problem. While requests not timing out is a problem, mostly those requests will not block operation, but instead will cause some missed events or potentially miss logging error conditions, for example.
Thank you, Ryan
Hi @kinyoklion !
Sorry, I didn't know I should've switched to the scoped version; I saw the last release of this one being last month and thought it was the official one.
Now I don't know if the example applies with the other package, but the scenario I had was a call to .variation
inside a lambda, so I set the timeout with a just a couple of seconds (as the lambda should respond fast); if, for some reason, your API is down, or the request doesn't connect, or my local connection when testing is slow, since timeout is not executed, the request keeps going until it actually fails, without emitting error
, and my code would be still await
ing the call.
Let me know if you want me to create the issue in the other repo.
Thanks!
Hey @homer0,
Thanks for the info. We can go ahead and work from the issue here.
Thank you, Ryan
Filed internally as 211416
Any updates on this issue? I think it may be related to some truly ridiculous request times I'm seeing from my applications. This was mostly stable for me until the recent LaunchDarkly outage on 2/22. Since then I have seen a lot of requests take an excessive amount of time.
@aballman These are streams and it is expected that they remain open forever. Incremental updates are delivered over the stream. These requests are not intended to timeout unless they are not receiving data on a regular interval, but is very different from a REST request.
I am not sure what this trace is demonstrating, but if all of these streams are from a single SDK instance, then that would be representative of some kind of problem, but unrelated to this issue.
I am not sure what this trace is demonstrating, but if all of these streams are from a single SDK instance, then that would be representative of some kind of problem, but unrelated to this issue.
OK, fair enough. Appreciate the quick response. Just seemed odd that this was new-ish behavior. I have some alerts on p90 request time of outbound connections that has been quiet for ages and then this recently came up. If it's unrelated then I'll figure something else out. Thanks!
@aballman That is interesting. I am unsure why that would be unless something was somehow excluding these, because it understood that it was an incremental transfer, and now somehow that isn't correctly being identifier.
An individual SDK should have one of these streams open at all times, and it basically reconnects when our back-end deploys occur.
If an individual SDK has multiple concurrent streams, then something has gone wrong.
Thank you, Ryan
Describe the bug The timeout in the requests is being ignored because there's no listener for the
timeout
event. The problem is here. The code is listening forerror
, but when a request times out, it emits atimeout
event, that it's being ignored, so until the server fails to respond, the request will continue.I copied the
httpRequest
into an isolated scenario and tried adding the following things and it seems to work:To reproduce Set the timeout to a very low value and throttle your network connection.
Expected behavior The
timeout
event will be handled, the connection terminated, and the promise rejected.SDK version Latest
Language version, developer tools Node 16 and 18
OS/platform MacOS 12