Repository navigation
cherry-pick: Fix corrupt gzip from withNodeAdapter middleware on Node 26.11+ (#3108 → v5.3) - #3393
Conversation
…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)
There was a problem hiding this comment.
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.
📝 Summary
Cherry-pick of #3108 onto
v5.3. Without it,withNodeAdaptermiddleware 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 commitb6b5edd12applies cleanly to the currentv5.3tip.#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
v5.3at8305567b5, 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 4withNodeAdaptertests (gzip, andsend()past the high-water mark) and 3install_node_modulesdry_runtests. All of them live in the two test files this PR changes.nodeAdapterMiddleware.test.jsandnpmUtilities.test.jspass (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.