GeezerVison is showing github.com — Retry patch API connections reset mid-handshake by mikolalysenko · Pull Request #610 · SocketDev/socket-patch
Conversation
Problem
The patch API's JSON retry loop (api::retry, ApiClient::send_json_request) retries only HTTP 429 and 503. Any transport error is final on the first failure. CI's e2e legs and the per-package-manager compatibility matrices call the production patch API (patches-api.socket.dev). So when a load balancer resets one fresh connection, the CLI command fails and the whole leg goes red.
Evidence (CI history 2026-09-28 → 2026-10-02):
- Direct hit: run 37035216705 (attempt 1, job
e2e (ubuntu-latest, e2e_safety_pnpm), branch from Pipenv recognizes hosted PyPI patch URLs with two private grammars that disagree with the shared one #563). socket-patch get failed with:
Error: Network error: error sending request for url (https://patches-api.socket.dev/patch/view/80630680-…): client error (Connect): Connection reset by peer (os error 104).
It went green on re-run of the same SHA.
- PDM matrix:
pdm-compatibility.yml had 15 failed runs and 14 re-runs in 200 runs. That is 17 failed native legs in 15 runs over the last 2 days, each with exactly one case failing and a different PDM version / shape each time. The failing checks are appliedExactlyOne ×7, rescanIdempotent ×5, refusalCodeReported ×4 and relock-rescan ×1. Each one needs a patch-API call that returned nothing usable. On refused-lock cells, the scan reported no refusal code and took 23–27 s where its siblings took 2–8 s. The harness prints only check names, and the artifacts can't be downloaded from this sandbox, so the PDM link is likely but not proven from logs.
- Same signature elsewhere: the Poetry (Fix Poetry matrix flakes on PyPI/patch API transport blips #596), Pipenv (Fix Pipenv matrix flake on 503/DNS transport blips #419), vlt (Fix vlt e2e legs failing when api.socket.dev blips #448) and Bun (Fix Bun backtest flake: retry cells on vex-step transport errors #565) harnesses each grew their own transport retry around CLI
error sending request for url ( failures. This fixes the shared cause in the client.
Root cause
send_json_request turned every reqwest send error into ApiError::Network at once. A connection the peer resets during TCP connect or the TLS handshake is a transient blip, and the request has not been sent yet. It was still treated like a real answer.
Fix
- New
api::retry::is_retryable_transport returns true only for a connect-phase error (is_connect() && !is_timeout()) whose cause chain holds an I/O ConnectionReset, ConnectionAborted or UnexpectedEof.
- The connector wraps the OS error in an
Other io::Error, and io::Error::source skips that inner error, so the walk also looks through get_ref().
send_json_request retries such an error under the existing policy:
- the same
max_retries (3, SOCKET_API_MAX_RETRIES, 0 = off)
- the same jittered exponential backoff
- the same 60 s run-wide window
- the final error message is unchanged
- This covers
get_json (search, patch view), post_json and the proxy batch POST. No request bytes have gone out at connect time, so retrying a POST is safe.
- Still final at once, as before:
- connection refused. Offline runs and the many tests that point the CLI at a dead port still fail fast.
- connect and read timeouts. The Bound patch API connects and stalled reads (#570) #581 "a stall is not repeated" rule holds.
- DNS and TLS-certificate errors.
- any failure after the request was sent.
fetch_blob and the vendoring service are untouched. The vendoring service already retries transport errors.
Proof
- New
a_connection_reset_mid_handshake_is_retried_then_reported in crates/socket-patch-core/tests/api_retry_e2e.rs uses a local TLS endpoint that RSTs every connection after the ClientHello. That reproduces the CI error locally: hyper_util Error(Connect, Custom{Other, Os{ConnectionReset}}). The test asserts:
- 4 connections (1 attempt + 3 retries) for the per-package GET and for the proxy batch POST
- 3 jittered backoff waits
- the same
Network error: … text
- exactly 1 connection with retries off
- New
a_refused_connection_is_not_retried asserts no retry for a refused connection.
- Before the fix, the reset test fails with
left: 1, right: 4, so the error was not retried. After the fix, api_retry_e2e passed 50/50 runs in a loop (17 tests, about 0.1 s).
cargo test -p socket-patch-core --lib api::: 265 passed.
socket-patch-cli binaries cli, get, apply, scan, scan_api_retry_e2e, remove_rollback_api_overrides, cli_get_silent, cli_scan_silent: 372 passed, 0 failed. These exercise the dead-port, proxy-fallback and retry paths.
cargo clippy -p socket-patch-core --all-targets -- -D warnings gives no findings in the touched files. Its 9 existing errors in other files are the same on main with this toolchain.
rustfmt was run only on the touched files.
- Not done: a live run against patches-api.socket.dev, which is unreachable from this sandbox. This PR's own e2e and compatibility legs exercise the production path.
Where tests run
No test is removed or moved. The new tests are in the existing api_retry_e2e binary, which ci.yml's test job runs. No workflow or job names change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01F1QpyLyaVEx88TT3UwW1L1
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The JSON retry loop repeated only 429/503; any transport error was
final on the first failure. CI e2e legs talk to the production patch
API, and a load balancer resetting a fresh connection ("client error
(Connect): Connection reset by peer") failed the whole leg, e.g. the
pnpm safety e2e on #563.
A connection reset, aborted or closed while being established is now
retried under the existing budget, backoff and run-wide window. No
byte of the request has gone out at that point, so it is safe for the
batch POST too. Refused connections, timeouts, DNS and TLS certificate
errors, and failures after the request was sent stay final at once, so
dead-port tests and stalled reads behave as before.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01F1QpyLyaVEx88TT3UwW1L1
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit efb631f. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[burn-down agent] Labeled Ready for review at efb631fae252b126616ab162d007468512913f42.
- CI: 332/339 check runs green on this head (7 skipped by path/matrix filters), 0 failing.
- Bugbot: reviewed this exact head, no findings; no unresolved review threads.
- Mergeable, up to date with
main @ 045d7ec.
- Reviewer focus:
api/retry.rs classification of connection-reset-during-handshake as retryable, and that it composes with the connect/read bounds from Bound patch API connects and stalled reads (#570) #581.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Reviewed efb631fae252b126616ab162d007468512913f42. No actionable findings; ready to merge from a code-review perspective.
Validated the transport classifier, fresh request construction, shared HTTP/transport retry budget and window, and unchanged timeout/post-send error boundaries. Locally, 28 distinct tests passed: 17 API retry tests, 6 timeout tests, and 5 independent probes covering post-send resets/truncated bodies, TLS EOF, exhausted windows, mixed failures, and timeouts with retries enabled.
Exact-head CI has 332 successful checks and 8 skipped, with no pending or failing checks. Bugbot is clean, no review threads remain unresolved, and the branch merges cleanly with current main (045d7ec7).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 2, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
ci-janitor
Opened by the CI janitor routine (flakes, redundant tests, CI perf)
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|