Repository navigation
fix: Reconnect the stream after non-HTTP read errors - #128
Open
kallevmercury wants to merge 6 commits into
Open
kallevmercury wants to merge 6 commits into
kallevmercury wants to merge 6 commits into
Conversation
The socket reset and split UTF-8 tests fail on this commit. The streaming thread exits after one connection, without retrying or logging, so the SDK stops receiving flag updates.
A socket reset, TLS error, or a UTF-8 character split across two reads threw a non-HttpException from the body read, which ended the streaming thread with no log and no reconnect. The body read now decodes UTF-8 incrementally, and other read errors are logged and rethrown as an HttpException so the existing backoff and reconnect apply. Async exceptions still pass through so close stops the thread.
backoffCeiling is the pure part of calculateDelay, with no behavior change. The new test fails on this commit: at attempt 46 the Int product overflows to a negative ceiling, so retries run with no delay.
… delay After 45 failed attempts at the default 1 s initial delay, the backoff product overflowed Int and the SDK retried with no delay. Capping the exponent at 15 still reaches the 30 s maximum for any initial delay of 1 ms or more.
There was a problem hiding this comment.
🟡 Changes recommended
The retry backoff multiplication can overflow Int on 32-bit runtimes before the cap is applied, producing negative delays and failing the new regression test.
1 open issue
What changed in this PR
This PR improves streaming data source recovery so interrupted connections do not permanently stop flag updates.
Changes:
- Retries non-HTTP stream read errors while preserving asynchronous cancellation.
- Decodes UTF-8 incrementally to handle characters split across reads.
- Caps the backoff exponent and adds regression tests for reconnection, decoding, shutdown, and retry delays.
| File | Description |
|---|---|
test/Spec/Streaming.hs |
Adds scripted stream tests and a backoff regression test. |
src/LaunchDarkly/Server/Network/Streaming.hs |
Adds incremental decoding, read-error recovery, and capped backoff calculation. |
Review effort: Balanced
kallevmercury
marked this pull request as ready for review
November 10, 2026 23:39
The new assertion fails on this commit: backoffCeiling maxBound 1 wraps to -1000. The same Int overflow hits the default 1 s delay on 32-bit runtimes.
…erflow The capped exponent still let the Int product wrap on 32-bit runtimes at the default 1 s delay, and on any runtime for a very large initial delay.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements
Related issues
None.
Describe the solution you've provided
Apps on the default streaming data source used to stop getting flag updates until restart after any of these stream read errors:
tryHTTPonly catchesHttpException, so these errors ended the streaming thread with no log or reconnect, and an uninitialized app never initialized. The fix has two parts:streamDecodeUtf8).closestill stops the stream.The retry delay also used to overflow
Intafter 45 failed attempts at the default 1 s delay, so retries ran with no wait. Computing the delay inIntegerkeeps it at the 30 s maximum.Describe alternatives you've considered
Lenient UTF-8 decoding would avoid the error but replace the split character with U+FFFD, corrupting the flag. The handling covers only the body read, so an invalid SDK key still stops the stream instead of retrying forever.
Additional context
The tests in
test/Spec/Streaming.hsrun the real streaming thread against scripted reads from a fake connection. Each test lands one commit before its fix: the reset and UTF-8 tests fail on commit 1, the backoff test on commit 3.I have also replicated this condition on the other LD SDKs, and they log an error and attempt to reconnect:
[WARNING] ldclient.util: Error on stream connection: ("Connection broken: ConnectionResetError(54, ...)") - will retry in 0.1s.Received I/O error (aborted) for streaming request - will retry, then Will retry stream connection in 63 milliseconds.