Repository navigation
Stop the PATCH read-back test racing a closing socket - #129
Merged
Merged
Conversation
patch_with_an_empty_body_of_unknown_length_reads_the_resource_back failed intermittently with "Update succeeded, but reading it back failed: Network error: error sending request". Its raw server closes every connection, but the chunked reply did not say so, so reqwest pooled the connection and could send the read-back GET down it as the server closed it. A 50 ms pause before the server's close reproduces the failure every time; with Connection: close on the reply it passes. The client code is unchanged: Graph does not close a keep-alive connection right after answering. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
api::client::tests::patch_with_an_empty_body_of_unknown_length_reads_the_resource_backfailed once while I was verifying #119 locally:Cause: the test's raw HTTP server closes every connection after replying. Its chunked
200did not sendConnection: close, though, so reqwest put the connection back in its pool and sent the read-back GET down it. If the server's close landed first, that GET failed.Reproduced: adding a 50 ms sleep before the server's
shutdown()makes the test fail every time, with the same error.Fix: the chunked reply now sends
Connection: close, as the other reply in the test already did. With the 50 ms sleep still in place, the test passes. This changes only the test: the client code is correct, and Graph does not close a keep-alive connection right after answering.Separately, this test passed 300 isolated runs and 40 full-suite runs on main, so the race is rare. CI has never hit it.
🤖 Generated with Claude Code