Bound patch API connects and stalled reads (#570) by mikolalysenko · Pull Request #581 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-02T16:58:34Z

Fixes #570.

Both authenticated and header-free ApiClient clients now use a shared transport policy: a 10-second connect bound and a 60-second idle read bound. Stalled proxies, half-open connections, and incomplete response bodies fail instead of hanging indefinitely.

Behavior and implementation

ApiTimeouts in api/retry.rs supplies the named defaults and applies them to both client builders. with_api_timeouts provides a test override.
The read timeout restarts after each chunk. Blob/diff streams that keep delivering data do not receive an added total-transfer deadline or size cap.
JSON body timeout/transport errors are classified as ApiError::Network, preserving their underlying cause. Completed malformed JSON remains ApiError::Parse.
The shared JSON classifier covers normal responses, proxy batch search, and vendor package-reference responses. Existing vendor retry hints and budgets remain intact.
CHANGELOG records the fix. No environment override is added.

Validation

62 focused API tests pass: 6 timeout/JSON, 15 API retry, 4 binary error classification, 19 blob edges, and 18 vendor retry tests. The new partial-JSON body-stall regression fails before the correction, on authenticated and proxy paths. Malformed-JSON and vendor retry behavior remain covered.
Those API tests ran on 43269f46; the final 202e4774 keeps the API source and timeout tests byte-identical and changes no runtime dependency versions.
The branch incorporates current main 203e092b. Its benchmark fixture now requires both package.json and yarn.lock for hosted Berry pinning, matching the descriptor-resolution change already on main.
The strict benchmark expectation correction is cherry-picked with provenance from isolated commit 329b1467 in Stream ZIP member hashes during vendored verification #587. No ZIP verification changes are included. All 29 benchmark harness tests pass; the identical harness already passed against the same main baseline.
The earlier scan-performance failure was an invalid fixture expectation on both baseline and head; all 37 measured comparisons were unchanged. Final-head CI is complete: 332 successful checks, 6 skipped; no failures or pending checks. Bugbot passed, there are no unresolved review threads, and the PR is mergeable. No full local workspace or timing run was repeated.

Risk and scope

An API response that remains silent for more than 60 seconds now fails; long computations before the first response byte must fit that idle bound. Blob/diff retry policy, merging the retry systems, and streaming blob/diff implementation (#571) remain outside this change.

Note

Medium Risk
Silent API responses longer than 60s now fail; environments with very slow first-byte latency may see new network errors where commands previously hung.

Overview
Patch API traffic (scan, get, blob/diff downloads, vendoring) no longer blocks indefinitely when a proxy or connection stalls. Both authenticated and public-proxy HTTP clients share ApiTimeouts: 10 s to connect and 60 s of silence on an open socket before failing with ApiError::Network.

The read bound is per idle period, not a total transfer limit—streams that keep sending data still complete. Stalled or truncated JSON bodies are classified as network errors (not parse errors), including vendor package-reference responses where retry hints are unchanged. Tests can shorten bounds via with_api_timeouts.

The Yarn Berry benchmark fixture now expects hosted redirects to rewrite package.json as well as yarn.lock, matching descriptor-resolution pinning on main.

^{Reviewed by Cursor Bugbot for commit 202e477. Configure here.}

Assisted-by: Claude Code:claude-opus-5-5

scan, get, apply and vex talk to the patch API through two reqwest clients that had no connect or read timeout, so a stalled proxy, load balancer or half-open connection hung the run until the CI job was killed. Both clients now take one named ApiTimeouts policy (10 s connect, 60 s of silence on an open connection). A stall fails as ApiError::Network, which the proxy fallback already handles; a download that keeps streaming is never cut off. Fixes #570 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-02T17:09:08Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-02T18:12:40Z

bugbot run

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-02T18:23:15Z

[burn-down agent] Labeled Ready for review at 826833aa34489ffed7c632724211a9ece3933c48.

CI: 331/337 check runs green on this head (rest skipped by path/matrix filters), 0 failing. Mergeable; 7 commits behind main, only CHANGELOG.md overlaps and it merges cleanly.
Bugbot: re-requested (the earlier run was cancelled) and it reviewed 826833aa34 with no issues found; no unresolved review threads.
Reviewer focus: the chosen bounds (10 s connect, 60 s idle read, no total deadline) in api/retry.rs.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-02T18:34:29Z

Reviewed final head 202e47744b96149f64a1f69a69c35e97420627a3.

The reproduced P2 JSON body-stall issue is fixed: all three JSON readers retain timeout/transport failures as Network with the underlying cause; completed malformed JSON stays Parse, and vendor retry behavior is preserved. 62 focused API tests passed.

The subsequent scan-performance failure was a stale Berry fixture, not a measured slowdown. Both main and this PR correctly rewrite package.json plus yarn.lock, while the fixture expected only yarn.lock; all 37 measured comparisons were unchanged. The branch now includes current main and reuses the isolated correction from #587, preserving provenance and requiring both files. The identical harness previously passed against the same baseline, and 29 benchmark tests pass locally. API source/tests are unchanged from the tested timeout fix.

Ready to merge as-is from this review. Final-head CI is complete: 332 successful checks, 6 skipped; no failures or pending checks. Bugbot passed, there are no unresolved review threads, and the PR is mergeable. No full local workspace or timing run was repeated.

Mikola Lysenko (mikolalysenko) · 2026-10-02T18:40:25Z

BugBot review

Please review the JSON body timeout fix at 43269f46dcf0b6b663dfdd28bd38b48c1f1bd6bb. The reproduced issue and 62 passing focused tests are documented in the review comment above.

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-02T18:54:06Z

[agent] The scan performance check fails at 43269f4, and the cause is not in this PR. Job log.

What fails: the yarn-berry/hosted and yarn-berry/rescan scenarios are marked INVALID for both binaries, including the base binary built from main @ 203e092. The error is redirect.rewrittenFiles: got ["package.json", "yarn.lock"], want ["yarn.lock"]. The other 38 scenarios are ≈, so this PR shows no timing change.
Why it isn't this PR's: the bench suite (Add a scan benchmark suite and a CI performance gate #485, a79de97) landed before Fix yarn berry hosted pin leaking npm auth (#404) #465 (203e092), which changed yarn berry hosted mode so it also writes package.json. The fixture expectation at crates/socket-patch-bench/src/fixtures/npm.rs:626 still says &["yarn.lock"]. The head rescan's got [] probably comes from the base run's leftover fixture. This PR doesn't touch yarn or the bench.
Fix: I couldn't find one in any open PR. Proposed patch, which belongs in its own PR rather than this one:
```
-    Ok(fixture(&g, g.patches(true), &["yarn.lock"], &[]))
+    Ok(fixture(&g, g.patches(true), &["package.json", "yarn.lock"], &[]))
```
(in yarn_berry in crates/socket-patch-bench/src/fixtures/npm.rs). Please check the rescan scenario after that change. Every PR based on current main will fail this gate until it lands.
Re-run: not done. The failure is deterministic on main's own binary, so a re-run would fail the same way.

Generated by Claude Code

…tch-581-benchmark-20261002

(cherry picked from commit 329b146)

Mikola Lysenko (mikolalysenko) · 2026-10-02T19:04:39Z

BugBot review

Please review final commit 202e47744b96149f64a1f69a69c35e97420627a3. The JSON timeout fix is unchanged; this head also incorporates current main and the verified one-file Berry benchmark expectation correction. The PR description and review comment reflect the final state.

cursor

✅ 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 202e477. Configure here.}

Start refactor for #570

84f60f8

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Oct 2, 2026

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 2, 2026

The patch API client has no request timeout, so scan, get and apply hang forever on a stalled server #570

Closed

3 tasks

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 2, 2026 17:09

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 2, 2026

refactor: #581 for #570, C02 in PR

9116e95

cursor Bot reviewed Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026

Classify stalled JSON response bodies as network errors

43269f4

Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026

cursor Bot reviewed Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added 2 commits October 2, 2026 14:59

Merge commit '203e092bdb91f573df9d0120eae2c2004a50d358' into codex/wa…

cfcce98

…tch-581-benchmark-20261002

test: align Berry benchmarks with descriptor resolutions

202e477

(cherry picked from commit 329b146)

cursor Bot reviewed Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026

Tanmay Singla (Tanmay182003) approved these changes Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) merged commit 045d7ec into main Oct 2, 2026
339 checks passed

Mikola Lysenko (mikolalysenko) deleted the arch-refactor/570-api-timeout branch October 2, 2026 19:59

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 2, 2026

refactor: #581 merged, C02 fixed

56212bc

This was referenced Oct 2, 2026

Share rollback artifact retention across remove and rollback #600

Open

Fix hosted scan from a workspace member pinning nothing or the wrong files (#590, #417) #598

Open

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 2, 2026

Retry patch API connections reset mid-handshake #610

Open

Navigation Menu

Bound patch API connects and stalled reads (#570) - #581

Bound patch API connects and stalled reads (#570)#581
Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
arch-refactor/570-api-timeout

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behavior and implementation

Validation

Risk and scope

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading