[maxdeviant]

This PR makes it so a tarball download that stopped receiving data is retried rather than stalling indefinitely.

We've been seeing a number of stalls in our CI when using the pnpm/setup Action:

Run pnpm/setup@703c52620218391530e48b9e8870d5c0082e1b9b
  with:
    version: 11
    runtime: node@24
    install: false
    dest: ~/setup-pnpm
    cache: false
    working-directory: .
    require-lockfile: false
  env:
    CARGO_TERM_COLOR: always
    NSC_CONTAINER_REGISTRY: nscr.io/REDACTED
Installing pnpm...
  Downloading pnpm 11.28.6 from the npm registry
  ==> Downloading pnpm 11.28.6
DOMException [TimeoutError]: The operation was aborted due to timeout
    at new DOMException (node:internal/per_context/domexception:76:18)
    at Timeout._onTimeout (node:internal/abort_controller:210:9)
    at listOnTimeout (node:internal/timers:635:17)
    at process.processTimers (node:internal/timers:571:7)

---

Note

Everything below this line is commentary from Claude.

Follow-up to #62. That PR retries a tarball download whose connection was reset, but deliberately does not retry one that runs out its 15-minute budget, since the budget is sized for a slow connection and retrying would multiply the wait.

That leaves a gap: a registry that accepts the connection and then stops sending (either partway through the body, or before any response at all) still hangs the install for the full 15 minutes and then fails. We hit this in CI through pnpm/setup:

17:06:19  ==> Downloading pnpm 11.28.6
17:21:20  DOMException [TimeoutError]: The operation was aborted due to timeout

The same download normally takes about 2.5 seconds, and a re-run succeeds.

Change

Each tarball attempt now has a stall timeout alongside the overall budget:

• A watchdog timer starts with the request and resets on every chunk received. If 30 seconds pass with no data, it aborts the attempt.
• Its signal is combined with the existing AbortSignal.timeout via AbortSignal.any, so it covers both waiting for the response and reading the body.
• It aborts with a plain Error ("no data received for 30s") rather than a TimeoutError, so the existing isRetryableError treats it as an interrupted connection and retries it. The overall 15-minute timeout is still not retried, as fix: retry a registry or tarball request that failed transiently #62 intended.
• A slow download that keeps making progress is unaffected, since every chunk resets the timer.

downloadTarball takes an optional stallTimeoutMs (defaulting to 30s) so the tests can use a short one. Metadata requests are unchanged, since their 30-second overall timeout is already retried.

Tests

test/stalledDownload.test.ts covers both shapes of stall against a local server that leaves the first request open:

• The server sends half the body, then nothing more.
• The server never sends a response at all.

In both cases the download is retried and completes on the second request. Without the stall signal passed to request(), both tests hang until the test runner's timeout.

Summary by CodeRabbit

• New Features

  • Tarball downloads now detect transfers that stop making progress and abort them after a configurable timeout, defaulting to 30 seconds.
  • Active downloads continue uninterrupted while data is still arriving, even if the transfer is slow.
  • Interrupted downloads can automatically retry, improving reliability when a transfer stalls.

• Bug Fixes

  • Improved handling of stalled downloads during both connection setup and partially completed transfers.

[vercel]

@maxdeviant is attempting to deploy a commit to the pnpm Team on Vercel.

A member of the Team first needs to authorize it.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[netlify]

✅ Deploy Preview for getpnpm ready!

Name	Link
🔨 Latest commit	e485f50
🔍 Latest deploy log	https://app.netlify.com/projects/getpnpm/deploys/6ad7aa5b725684856448cd0d
😎 Deploy Preview	https://deploy-preview-73--getpnpm.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 00a4d338-acbd-4cec-937e-6b92f47b66ba

📥 Commits

Reviewing files that changed from the base of the PR and between b7cf47f and e485f50.

📒 Files selected for processing: 2

• get-pnpm/src/registry.ts
• get-pnpm/test/stalledDownload.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

⚠️ CI failures not shown inline: 1

Commit Status: Vercel: Vercel

Conclusion: failure

Authorization required to deploy.

🧰 Additional context used

🪛 ast-grep (0.45.3)

get-pnpm/test/stalledDownload.test.ts

[warning] 24-38: Use https protocol over http
Context: http.createServer((req, res) => {
requests++
if (silencesRemaining > 0) {
silencesRemaining--
return
}
res.writeHead(200, { 'content-type': 'application/octet-stream', 'content-length': body.length })
if (stallsRemaining > 0) {
stallsRemaining--
// Left open, like a registry that stopped sending partway through.
res.write(body.subarray(0, body.length >> 1))
return
}
res.end(body)
})
Note: [CWE-319] Cleartext Transmission of Sensitive Information. Security best practice.

(https-protocol-missing-typescript)

[warning] 61-61: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(dest)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

[warning] 71-71: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(dest)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

🔇 Additional comments: 1

get-pnpm/test/stalledDownload.test.ts: 1

12-12: 🩺 Stability & Availability

The review comment is refuted. The repository explicitly separates the published package minimum from the test-runner requirement: Node.js 22.13 supports the package, while the test runner requires Node.js 22.18 or newer. The test script follows that documented requirement, so it does not need to run on Node.js 22.13.

📝 Walkthrough

Tarball downloads now use a configurable no-progress timeout, set to 30 seconds by default. The watchdog resets when data arrives and works with the existing overall timeout. Tests cover retries after stalls before headers and during the response body.

Changes: Tarball stall handling

Layer / File(s)	Summary
Watchdog and download integration get-pnpm/src/registry.ts	TarballOptions.stallTimeoutMs configures the no-progress timeout. A watchdog resets when chunks arrive, and its abort signal is combined with the overall timeout signal.
Stall retry tests get-pnpm/test/stalledDownload.test.ts	Local HTTP tests simulate a silent response and a mid-body stall. Both tests verify that the download completes after two requests.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e485f

No confirmed issue currently blocks merging. The behavior of an overall timeout during body streaming remains unverified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: retrying tarball downloads after they stop receiving data.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{I’m a rabbit watching bytes arrive,
A quiet stream gets time to revive.
If the chunks pause, the timer will call,
Then retries bring the tarball through all.
I nibble my carrot and hop along.}

_{Comment @coderabbitai help to get the list of available commands.}