Skip to content

Ride out crates.io blips in required cargo e2e legs - #1459

Merged
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-janitor/cargo-e2e-net-retry
Oct 13, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-janitor/cargo-e2e-net-retry

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The cargo 1.93.1 lock-v4 (ubuntu-latest) leg (cargo-vex-matrix, which runs on PRs and in the merge queue) failed on PR #1449, run 38307581268 attempt 1. It passed when the same SHA was re-run. The failing test was e2e_vendor_cargo_build::cargo_rollback_after_agent_to_vendored_restores_cache_behind_cargo_vendor_dir, in its baseline fixture build:

baseline `cargo build` failed (crates.io unreachable?):
error: failed to download from `https://static.crates.io/crates/cfg-if/1.0.5/download`
  [7] Could not connect to server (Failed to connect to static.crates.io port 443 after 0 ms ...)
 (and SOCKET_PATCH_CARGO_E2E_REQUIRED is set, so this leg must not skip)

Each of the ~120 tests in these suites does a fresh-CARGO_HOME fetch from crates.io. A short network blip fails the leg, and in the merge queue it would evict the entry and cost a full rebuild.

Root cause

Cargo treats a connect failure as spurious and retries it, but the default net.retry = 3 gives up after about 12s. The test took 13s. A blip longer than that fails the build. With REQUIRED=1 set, the harness turns that failed build into a hard failure.

Fix

cargo_e2e_matrix::cargo_command is the one helper every real-cargo suite uses (e2e_redirect_cargo_build, e2e_redirect_cargo_shapes, e2e_vendor_cargo_build, mode_migration_cargo, e2e_safety_cargo_build). When the leg is required, it now sets CARGO_NET_RETRY=10 on its children, which gives about 80s of cargo's own backoff. An ambient CARGO_NET_RETRY still wins. Optional local runs keep the default, so an offline machine still skips quickly. Only errors cargo itself classifies as transient are retried. Checksum, resolver and --locked failures still fail immediately.

The one test that needs a fast network failure (the old-cargo 127.0.0.1:9 proxy check in e2e_vendor_cargo_build.rs) builds its own Command with CARGO_NET_RETRY=0, so it is unaffected.

Proof

  • Retry window, with cargo fetch of cfg-if through a refused proxy (the same [7] Could not connect ... after 0 ms error): retry=3 fails after 12s (3 spurious warnings). retry=10 fails after 80s.
  • Simulated 20s blip (a local forwarder that starts accepting connections after 20s): retry=3 fails after 12s with the CI error. retry=10 succeeds after 21s (4 spurious retries, then Downloaded cfg-if v1.0.5).
  • SOCKET_PATCH_CARGO_E2E_REQUIRED=1 cargo test -p socket-patch-cli --test e2e_redirect_cargo_build --test e2e_redirect_cargo_shapes --test e2e_vendor_cargo_build --test mode_migration_cargo: 22 + 31 + 39 + 29 passed.
  • rustfmt --check is clean on the touched file. Clippy (-D warnings) reports nothing in the touched file. The same targets already fail clippy on unmodified main in tests/common/mod.rs and tests/prebuilt_common/mod.rs (11 errors either way).

No tests removed or moved.

Not covered: a 2-minute runner DNS outage (main push run 38182319501, rustup toolchain install stable failed 4 times on Temporary failure in name resolution). That is runner infrastructure, and no retry window here is meant to absorb it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GTcGpX2RVXH9BXATkDJLdm


Note

Low Risk
Test-harness-only change to env vars on spawned cargo processes; no production runtime or security impact.

Overview
Required cargo e2e legs now set CARGO_NET_RETRY=10 on child cargo invocations (via shared cargo_e2e_matrix::cargo_command) so transient crates.io connect failures get ~80s of Cargo’s built-in backoff instead of the default ~12s, which was flaking CI when SOCKET_PATCH_CARGO_E2E_REQUIRED forbids skips.

An explicit ambient CARGO_NET_RETRY still wins; optional local runs keep Cargo’s default for fast offline skips. A new REQUIRED_NET_RETRY constant and expanded helper docs describe the behavior.

Reviewed by Cursor Bugbot for commit 641708e. Configure here.


Generated by Claude Code

The cargo-vex-matrix lock-v4 leg failed on PR #1449 (run 38307581268,
attempt 1; green on re-run of the same SHA) when a fresh-CARGO_HOME
baseline `cargo build` could not connect to static.crates.io. Cargo
classifies that error as spurious, but its default net.retry of 3
gives up after ~12s, and SOCKET_PATCH_CARGO_E2E_REQUIRED turns the
failed fixture build into a failed leg. In the merge queue the same
blip evicts an entry.

Set CARGO_NET_RETRY=10 (~80s of cargo's own backoff) on the children
of cargo_e2e_matrix::cargo_command when the leg is required and the
caller has not set CARGO_NET_RETRY. Optional local runs keep the
default so an offline machine still skips quickly. Only transfers
cargo itself treats as transient are retried; checksum and resolver
failures still fail at once.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTcGpX2RVXH9BXATkDJLdm
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-janitor Opened by the CI janitor routine (flakes, redundant tests, CI perf) label Oct 13, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

1 similar comment
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 641708e. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Ready for review at 641708e77e334acc2e2f0e97e4667d280db65bfb.

  • CI: all green on this head (35 pass + 19 skipped), no failures.
  • Bugbot: reviewed 641708e, no new issues; no unresolved review threads.
  • Mergeable, no CHANGELOG.md changes.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: On required cargo e2e legs (SOCKET_PATCH_CARGO_E2E_REQUIRED), every cargo child spawned through the shared cargo_command() helper now gets CARGO_NET_RETRY=10, unless the environment already sets a non-empty CARGO_NET_RETRY. Cargo's default of 3 retries gave up after about 11s on a crates.io connect blip and failed the leg. With 10 it rides out about 80s. Test-only, one file.

Risk: low. Test harness only, and gated on required legs. It adds one env var and removes nothing, so hermetic scrubbing and the run_socket invocations are unchanged. The negative tests still fail as before: cargo doesn't retry a checksum mismatch or the wiremock 404 fallback, and no mock serves 5xx.

Look here:

Verified:

  • Read the full diff and every caller of cargo_command: e2e_redirect_cargo_build, e2e_redirect_cargo_shapes, e2e_vendor_cargo_build, mode_migration_cargo and e2e_safety_cargo_build. They all go through this helper. The one direct cargo_run, in e2e_safety, runs --offline --frozen and needs no network.
  • CI on 641708e: 35 success, 19 skipped of 54; ci-ok and clippy green. Cursor Bugbot passed. No unresolved review threads. CHANGELOG.md untouched.

Changes I made: none.

Open questions (non-blocking): a silently dropped connection could now take several minutes per transfer (11 tries x the 30s http.timeout) before it fails, so a real outage would show up as a job timeout instead of a clear "Could not connect". Also setting CARGO_HTTP_TIMEOUT lower on required legs would bound that, but it's optional.

Auto-merge (squash) is armed, so approving sends it straight to the merge queue.


Generated by Claude Code

Merged via the queue into main with commit 42266b6 Oct 13, 2026
54 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the ci-janitor/cargo-e2e-net-retry branch October 13, 2026 13:21
github-merge-queue Bot pushed a commit that referenced this pull request Oct 13, 2026
* ci(release): publish npm from release.yml, not publish-npm.yml

Every socket-patch npm package's trusted publisher is registered against
release.yml, and npm allows only one publisher per package. The 5.0.0
release dispatched publish-npm.yml, whose OIDC token named that file, so
npm rejected it with E401 before staging anything.

Move the npm leg back inline as release.yml's npm-publish job (same steps
as publish-npm.yml) and delete publish-npm.yml, so the existing
registration keeps working. Add an `npm-retry-version` input that skips
the version gate, build, tag, GitHub release and crates.io jobs and
re-stages npm from an existing release's assets; this retries 5.0.0 now
and replaces the manual Publish npm dispatch. crates.io stays dispatched
through publish-cargo.yml, since crates.io allows several publishers per
crate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): fold 22 more PRs into the 5.0.0 notes

Cover the PRs merged to main after #1432's base (1779317..4cb3643):
Cargo source replacement and registry = "crates-io" (#338/#455, #386),
pnpm lockfile=false (#1074), berry removed descriptors (#1435), yarnrc
${VAR} reads (#1100), CRLF .npmrc append (#1196), Composer vendor-dir
cascade and global home (#439/#686, #586), Go +incompatible vex (#484),
gem moved BUNDLE_GEMFILE and commented Gemfile revert (#952, #988),
Poetry source = "pypi" (#1282) and Hatch path characters (#547).

No entries for #1411 (macOS fix to code #1393 added in this release;
the #1393 entry already describes the intended behavior), #1426 (perf
regression introduced and fixed within 5.0.0) or the CI/test/bench PRs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): cover #1419, #1434, #1440, #1442, #1444 in the 5.0.0 notes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): 5.0.1

Cut [Unreleased] into [5.0.1] — 2026-10-13 and stamp 5.0.1 with
scripts/version-sync.sh. The 5.0.1 notes cover main through 501c6d9
(#1449), i.e. every PR after the v5.0.0 tag (6ad6add): the npm
publish fix (5.0.0 never reached npm or crates.io, 5.0.1 is the first
v5 there), #1465, #759, #1460, #1462, #1445, #869, #1447, #1450,
#1467, and #1449 under Maintenance. CI/test-only PRs (#1448, #1452,
#1454, #1457, #1459) have no entries.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore(release): 5.0.1 notes for PRs merged since the cut

Covers #1469, #1470, #1474, #1475, #1478, #1479, #1480 and #1482; #1484 is
CI-only. The notes now cover main through 285a5e8.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants