Skip to content

cherry-pick: Fix corrupt gzip from withNodeAdapter middleware on Node 26.11+ (#3108 → v5.3) - #3393

Merged
kriszyp merged 1 commit into
v5.3from
cherry-pick/v5.3/pr-3108
Oct 21, 2026
Merged

kriszyp merged 1 commit into
v5.3from
cherry-pick/v5.3/pr-3108

Conversation

@kriszyp

@kriszyp kriszyp commented Oct 21, 2026

Copy link
Copy Markdown
Member

📝 Summary

Cherry-pick of #3108 onto v5.3. Without it, withNodeAdapter middleware sends corrupt gzip on Node 26.11+. The v5.3 Node 26 unit job also fails on every commit, which blocks a clean 5.3.3. The squash commit b6b5edd12 applies cleanly to the current v5.3 tip.

#3108 is milestoned v5.4. This backport is a release-scope call for 5.3.3. It ships a product fix: 5.3 users on Node 26.11+ get corrupt gzip from Node-adapter middleware.

✅ Verification

  • On v5.3 at 8305567b5, the Node 26 unit job fails the same 7 tests before and after cherry-pick: Fix replicated duplicate retries entering the audit walk (#3366 → v5.3) #3377. They are 4 withNodeAdapter tests (gzip, and send() past the high-water mark) and 3 install_node_modules dry_run tests. All of them live in the two test files this PR changes.
  • Locally on Node 24: nodeAdapterMiddleware.test.js and npmUtilities.test.js pass (33 tests). Node 26 coverage comes from this PR's CI.

Release target: v5.3 — backport for 5.3.3; #3108 is milestoned v5.4
Complexity: trivial

🤖 Generated with Claude Code (Claude Opus 5.5), posted via @kriszyp.

…return install_node_modules' npm report as an object on every npm (#3108)

* Keep piped chunks on a patched write() under Node 26.11, and parse npm's report behind its dry-run diff

Node 26.11.0's Readable.pipe fast path (nodejs/node#66182) calls _write() directly
for a byte-mode Writable destination, skipping write() overrides. withNodeAdapter's
response is a real PassThrough, so compression's res.write patch was skipped for
send(...).pipe(res): corrupt gzip on the wire, or a stalled body. On runtimes with
the fast path the response gives each pipe source a second, inert 'data' listener,
which keeps pipe on dest.write(), and removes it on unpipe.

npm before 11.20 prints the human dry-run diff ahead of its --json report, so
install_node_modules returned that output as an unparsed string; npm 11.20 dropped
the diff and the dry_run tests' match on it. Parse the report after any prefix and
assert on its added count.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Release the pipe guard before pipe's own unpipe cleanup, so a stalled source stays paused

pipe's cleanup resumes a source awaiting 'drain' while any 'data' listener
remains, so with the guard's listener still attached an unpipe or a destroyed
response left the source flowing into nothing and its buffered data was lost.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Guard only core Readable sources, which legacy pipe() never releases

Legacy Stream.prototype.pipe emits 'pipe' but never 'unpipe', and neither it nor a
userland pipe() takes Node's fast path, so the inert listener is skipped for them.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Skip the pipe guard for sources the fast path never takes, and release it from one constructor listener

Object-mode and decoded sources cannot take Node 26.11's pipe fast path, so the
inert listener only cost them a call per chunk. One 'unpipe' listener prepended in
the constructor replaces the per-pipe release closure and still runs ahead of each
pipe's own cleanup.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Tolerate an 'unpipe' emitted without a source

Core and readable-stream always pass the source, but a bare res.emit('unpipe') from
third-party middleware must not throw out of the guard's release.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Float the Node 26 unit-test leg to the newest 26.x, as integration tests do

Without check-latest, setup-node prefers a cached 26.x, on which the pipe
fast-path regression test passes with the guard removed.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Check which package a dry run would add when npm reports it

npm 11's dry-run report names the packages in its add entries; npm 10's only counts
them, so the name is asserted when present.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

* Release the pipe guard only from a Readable, matching where it attaches

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_013r4w8GFxG9nBkMVeRf7bbR
Dispatch-Task: main-red-kriszyp_harper_26a914501_38e57eff

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
(cherry picked from commit b6b5edd)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a workaround for Node 26.11's Readable.pipe fast path in NodeAdapterResponse to ensure that overridden write methods (such as those used by compression middleware) are not bypassed. It achieves this by adding an inert 'data' listener to byte-mode Readable sources on 'pipe' and removing it on 'unpipe'. Additionally, the PR updates npmUtilities to correctly parse JSON reports from the stdout of npm versions prior to 11.20, which print human-readable dry-run diffs before the JSON payload. Comprehensive unit tests have been added to validate both the stream piping behavior and the npm report parsing across different versions. No review comments were provided, and I have no additional feedback to offer.

@kriszyp
kriszyp merged commit 9f12427 into v5.3 Oct 21, 2026
57 of 58 checks passed
@kriszyp
kriszyp deleted the cherry-pick/v5.3/pr-3108 branch October 21, 2026 16:16
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