Skip to content

fix(snackager): check HTTP status and handle stream errors when fetching tarballs#673

Open
ide wants to merge 2 commits intomainfrom
@ide/fix-tarball-fetch-error-handling
Open

fix(snackager): check HTTP status and handle stream errors when fetching tarballs#673
ide wants to merge 2 commits intomainfrom
@ide/fix-tarball-fetch-error-handling

Conversation

@ide
Copy link
Copy Markdown
Member

@ide ide commented Feb 24, 2026

Why

fetchAndExtract pipes the fetch response body directly to a file without checking response.ok. If the npm registry returns a 404 or 500, the error response body gets written to package.tgz, and then targz.decompress fails with an opaque decompression error instead of a clear "tarball fetch failed with status 404" message. Additionally, the response.body stream's error event isn't handled, so a mid-download network failure (connection reset, timeout) leaves the promise unresolved with a partial .tgz file.

How

Check response.ok before piping and throw with the HTTP status if the request failed. Listen for the error event on response.body to catch mid-download failures and reject the promise. No cleanup of partial .tgz files is needed since the caller's finally block in fetchBundle.ts already rimrafs the parent directory.

Test Plan

yarn build and yarn test pass.

…ing tarballs

Why
===
`fetchAndExtract` pipes the fetch response body directly to a file without checking `response.ok`. If the npm registry returns a 404 or 500, the error response body gets written to `package.tgz`, and then `targz.decompress` fails with an opaque decompression error instead of a clear "tarball fetch failed with status 404" message. Additionally, the `response.body` stream's `error` event isn't handled, so a mid-download network failure (connection reset, timeout) leaves the promise unresolved with a partial `.tgz` file.

How
===
Check `response.ok` before piping and throw with the HTTP status if the request failed. Listen for the `error` event on `response.body` to catch mid-download failures and reject the promise. No cleanup of partial `.tgz` files is needed since the caller's `finally` block in `fetchBundle.ts` already `rimraf`s the parent directory.

Test Plan
===
`yarn build` and `yarn test` pass.
@ide ide requested a review from byCedric as a code owner February 24, 2026 21:05
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.

1 participant