Skip to content

fix(nix): retry nix commands that fail with transient network errors - #2995

Merged
mikeland73 merged 2 commits into
mainfrom
mikeland73/retry-transient-nix-errors
Oct 6, 2026
Merged

mikeland73 merged 2 commits into
mainfrom
mikeland73/retry-transient-nix-errors

Conversation

@mikeland73

@mikeland73 mikeland73 commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

In nightly run 37286584167, test-nix-versions (macos-latest, 2.30.2) failed because a nixpkgs tarball download got cut off during nix print-dev-env:

unpacking 'github:NixOS/nixpkgs/104240a7…' into the Git cache...
error: cannot read file from tarball: Truncated tar archive detected while reading data

Nix retries failed downloads, but not ones that fail partway through unpacking. Users can hit this too, not just CI, so devbox now retries these errors itself.

Changes

  • nix.Cmd has a new MaxAttempts field, defaulted from the new nix.Nix.MaxAttempts. If a command fails and its stderr matches a known transient error, devbox re-runs it with a short backoff (2s, then 4s) and prints one line explaining why. The transient errors are:
    • truncated or damaged tarballs
    • connection reset by peer
    • curl Failure when receiving data from the peer and Timeout was reached
    • HTTP error 500/502/503/504
  • Devbox sets Default.MaxAttempts = 3 in internal/nix. The zero value keeps today's behavior for other users of the public nix package.
  • Where stderr is read from:
    • Output: the exit error, which covers print-dev-env, eval, path-info, and similar.
    • CombinedOutput: the combined output.
    • Run with a caller-provided stderr, like nix build: the last 8 KiB, copied off as it streams to the caller's writer.
  • Cases that are deliberately not retried:
    • Stderr is a terminal. Nix's output passes straight through so its progress bar still renders. Interactive nix build won't retry, but print-dev-env will. Non-TTY environments like CI retry both.
    • Stdin isn't a file. The first attempt may have consumed it.
    • GitHub rate limits (403/429) and DNS failures. These won't clear within a few seconds, and retrying would only slow down offline use.

Test plan

  • New tests in nix/command_test.go use a fake nix script and cover:
    • retry then success for Output, CombinedOutput, and Run with a teed stderr
    • giving up after MaxAttempts
    • no retry for non-transient errors, a 403, retries disabled, and non-file stdin
    • stopping when the context is cancelled during the backoff
  • go test -race ./nix/, go test ./internal/nix/..., and golangci-lint all pass
  • Ran devbox install and devbox run against real Nix with a locally built binary, with stderr piped so the teed path ran
  • CI

🤖 Generated with Claude Code

Nix retries failed downloads, but not tarballs that get cut off while
being unpacked, and it gives up on server errors after a few quick
attempts. A single truncated nixpkgs download failed the nightly
cli-tests (test-nix-versions, macOS, Nix 2.30.2):

  cannot read file from tarball: Truncated tar archive detected while
  reading data

Add Cmd.MaxAttempts (defaulted from the new Nix.MaxAttempts) so a command
that fails with a known transient error (truncated/damaged tarballs,
connection resets, curl receive failures and timeouts, HTTP 500/502/503/504)
is re-run with a short backoff, and a one-line notice is printed.
Devbox sets Default.MaxAttempts = 3. The zero value keeps today's
behavior for other users of the nix package.

stderr is checked from the exit error (Output), the combined output
(CombinedOutput), or a tail buffer teed off a caller-provided stderr
(Run, e.g. nix build). When that stderr is a terminal it's passed through
untouched so Nix keeps its progress bar, so those commands aren't
retried. Commands with non-file stdin aren't retried either, because the
first attempt may have consumed it. GitHub rate limits and DNS failures
are deliberately not retried because they won't clear within seconds.

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

Copy link
Copy Markdown
Collaborator Author

@claude ptal

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Claude finished @mikeland73's task in 27s —— View job


Review of #2995: retry transient nix errors

I read the diff and the surrounding nix/command.go. I didn't run the tests or the linter. Overall this looks good: it's well scoped, the opt-in default keeps the public nix package unchanged, and the exclusions (403/429, DNS, terminal stderr) are sensible. I found nothing blocking, only the points below.

Points worth a look

  1. Stdin as *os.File doesn't mean it's replayable (nix/command.go, canRetry). os.Stdin is an *os.File, but when it's a pipe or TTY the first attempt may already have consumed part of it. A fully non-seekable stdin can't be re-read. The risk is low, since these errors usually happen before Nix reads stdin. Still, you could restrict retries to regular files, or to Stdin == nil plus seekable files. Alternatively, document the assumption in the comment.

  2. Partial stdout is duplicated on retry (run). If the caller sets c.Stdout to a writer, as Run does for streaming nix build, a failed attempt may already have written some output. A retry then writes it again. This is probably harmless for the transient errors listed, because they happen during fetch before output is produced. Even so, a short note on MaxAttempts would help: "Stdout may receive output from failed attempts."

  3. Stderr fallback can read stdout (run). stderr := out is only meaningful for CombinedOutput. For Output or Run with a terminal c.Stderr, no tail is installed. Then exitErr.Stderr is empty and out is stdout, which gets scanned for the transient strings. A stdout line containing, for example, "Connection reset by peer" could trigger a retry. It's an unlikely false positive. You could make it explicit by only using out when the call was CombinedOutput. For example, have runFunc return the stderr source, or pass a flag.

  4. Terminal check happens only for non-nil c.Stderr. With Output and Stderr == nil, Nix's stderr is captured and not shown live. That means no progress bar and no live errors during the first attempt. That matches the existing Output behavior, so it's fine. The PR description covers it.

  5. tailWriter allocation. t.buf = t.buf[len-max:] keeps the old backing array alive and grows it with each append. For a long nix build this holds on to more memory than 8 KiB, and each write may copy. Streaming a lot of stderr through it makes that wasteful. A small ring buffer, or copying into a fresh slice when trimming, would bound it. Not a correctness issue.

  6. Retry message goes to os.Stderr when c.Stderr is nil (run). That's reasonable. In --quiet or JSON-output modes it could pollute stderr, though stderr is usually fine for this.

Nits

  • The Truncated tar archive and Damaged tar archive entries match against the full stderr. That's good, because the error: line may not be last.
  • delay is attempt * retryDelay, giving 2s then 4s as documented. retryDelay being a package var is handy for tests.
  • Tests: the PR covers the main matrix. One missing case is a stderr-source test showing that a transient string on stdout alone (point 3) isn't retried, if you decide to tighten that.

Verdict: LGTM with the optional tightening in points 1–3. CI is still unchecked in the test plan.

Address review feedback on the retry logic:

- Only scan stdout for transient errors when it's combined with stderr
  (CombinedOutput), so a stdout line can't trigger a retry.
- Bound tailWriter's backing array to 8 KiB by shifting old bytes out in
  place instead of reslicing.
- Document that stdout may receive output from failed attempts and why
  file stdin is still considered safe to retry.

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

Copy link
Copy Markdown
Collaborator Author

@claude ptal — addressed points 1–3 and 5 in 2cb16e9 (point 1 documented rather than restricted, since nix build always passes os.Stdin).

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Claude finished @mikeland73's task in 16s —— View job


Re-review of 2cb16e9

I read the diff for 2cb16e9. I couldn't run go test because the sandbox denied approval for it, so none of this has been run.

Points 1–3 and 5 are addressed

  • Point 1 (stdin): Documenting it is fine. Your reasoning holds: nix build always passes os.Stdin, so restricting to regular files would disable retries for it. The canRetry comment now says why file stdin is accepted.
  • Point 2 (stdout from failed attempts): The new note on MaxAttempts covers it.
  • Point 3 (stdout scanned for errors): The switch in run is correct. It prefers stderrTail, then exitErr.Stderr, and falls back to out only when combined is true. For Output or Run with a terminal stderr, stdout is no longer scanned. Passing combined as a bool is simple and clear.
  • Point 5 (tailWriter): The in-place shift is correct.
    • drop is at most len(t.buf), because data is capped at maxLen.
    • The copy happens before the append, so cap stays bounded.
    • The return value is still the original len(data), as io.Writer requires.
    • TestTailWriterSmallWrites checks len == 8 KiB, cap <= 16 KiB, and that the tail is kept.

Remaining nit

  • I didn't see a test for the point 3 change. A fake nix that prints a transient string to stdout only and exits non-zero, run through Output, should be called once. The same check could be run with a terminal-style stderr through Run. Without it, a later refactor of that switch could bring the false positive back without any test failing.

Not verified

  • I didn't run go test -race ./nix/ or the linter. The CI box in the test plan is still unchecked, so please confirm CI is green before merging.

Verdict: LGTM. The only follow-up is the optional stdout-only test above.

@mikeland73
mikeland73 merged commit 3317b4d into main Oct 6, 2026
44 of 48 checks passed
@mikeland73
mikeland73 deleted the mikeland73/retry-transient-nix-errors branch October 6, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant