Skip to content

fix(registry): retry and validate upstream packument responses#17

Merged
jacekradko merged 7 commits intomainfrom
jacek/retry-upstream-proxy
Mar 31, 2026
Merged

fix(registry): retry and validate upstream packument responses#17
jacekradko merged 7 commits intomainfrom
jacek/retry-upstream-proxy

Conversation

@jacekradko
Copy link
Copy Markdown
Member

Summary

Fixes transient CI failures where pkglab add fails because the upstream npm registry proxy returns truncated/corrupted JSON packuments. Seen in clerk/javascript CI where npm install failed with:

npm error invalid json response body at http://127.0.0.1:16180/typescript
reason: Expected ':' after property name in JSON at position 4128770

Root cause: proxyToUpstream streamed the upstream response body directly to the client. For large packuments (~4MB for typescript), a network hiccup could truncate the stream, serving corrupted JSON to npm.

Fix:

  • JSON packument responses are now buffered and validated with JSON.parse() before serving. If the JSON is corrupted, the request is retried.
  • Tarballs continue to stream directly since npm verifies integrity via shasum.
  • Up to 2 retries with linear backoff (500ms, 1s) on network errors or corrupted responses.

Test plan

  • Typechecks clean (bun tsc --noEmit)
  • No new lint errors introduced

@jacekradko jacekradko requested a review from nikosdouvlis March 31, 2026 17:09
msw pulled in ~30 transitive deps (graphql, tough-cookie, yargs, etc.)
to test a single function that calls fetch. spyOn(globalThis, 'fetch')
does the same job with zero new dependencies.
@nikosdouvlis
Copy link
Copy Markdown
Member

Pushed a commit swapping msw for spyOn(globalThis, 'fetch') from bun:test - msw was pulling in ~30 transitive deps (graphql, tough-cookie, yargs etc) for one test file. All 10 tests still pass. Feel free to revert if you prefer msw tho, no strong feelings.

Also should we drop the per-attempt timeout from 30s to 15s? Worst case right now is 30s x 3 attempts = ~91.5s to block an install, which feels like a lot

@jacekradko jacekradko merged commit 68638f7 into main Mar 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants