Skip to content

test(node): drop Readable.toWeb crash tests now fixed in Node 26.10 - #100

Merged
dinwwwh merged 1 commit into
mainfrom
claude/node-26-test-failures-f04447
Sep 25, 2026
Merged

dinwwwh merged 1 commit into
mainfrom
claude/node-26-test-failures-f04447

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 25, 2026

Copy link
Copy Markdown
Member

The Node 26 CI job is green again. Node 26.10 fixed the uncatchable ERR_INVALID_STATE crash when a Readable.toWeb stream is cancelled mid-upload (nodejs/node#62773). The two tests that expected the plain adapter to crash started failing, so they are removed. toWebReadableStream keeps working as before, and its doc comment now says to switch back to Readable.toWeb once every supported Node release has the fix.

Fixes

  • test_node_matrix (26) no longer fails with expected 0 to be greater than 0 on Node 26.10.
  • The tests showing toWebReadableStream survives aborted HTTP/1 and HTTP/2 uploads are unchanged and still run on every Node version.

Context

Testing

  • packages/node/src/utils.test.ts passes on Node 22.23, 24.21, 26.7 and 26.10.
  • Lint and type check pass.

Node 26.10 fixed the uncatchable ERR_INVALID_STATE crash when a
Readable.toWeb stream is cancelled mid-upload (nodejs/node#62773), so
the tests asserting that the bare adapter crashes started failing on
Node 26. Remove them and note the upstream fix on toWebReadableStream,
which stays until every supported Node release has it.
@pkg-pr-new

pkg-pr-new Bot commented Sep 25, 2026

Copy link
Copy Markdown
@standard-server/aws-lambda

npm i https://pkg.pr.new/@standard-server/aws-lambda@100

@standard-server/core

npm i https://pkg.pr.new/@standard-server/core@100

@standard-server/fastify

npm i https://pkg.pr.new/@standard-server/fastify@100

@standard-server/fetch

npm i https://pkg.pr.new/@standard-server/fetch@100

@standard-server/node

npm i https://pkg.pr.new/@standard-server/node@100

@standard-server/peer

npm i https://pkg.pr.new/@standard-server/peer@100

@standard-server/shared

npm i https://pkg.pr.new/@standard-server/shared@100

commit: 6b47004

@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/node-26-test-failures-f04447 (6b47004) with main (cc041e5)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • Removed the two canary tests — utils.test.ts no longer asserts that a bare Readable.toWeb crashes (ERR_INVALID_STATE) on aborted HTTP/1 and HTTP/2 uploads. They were written to fail once Node shipped a fix.
  • Simplified runUploadServer — the wrap parameter, stopOnCrash flag, and bare/wrapped helpers are gone; the helper now always spools via toWebReadableStream, and the crashes === [] / handled === 25 assertions are unchanged.
  • Updated the doc comment on toWebReadableStream — records that the crash is fixed upstream in Node 26.10 and when to retire the workaround.

I verified the load-bearing upstream claim: nodejs/node#62773 ("stream: fix ERR_INVALID_STATE when cancelling Readable.toWeb()") merged into main on 2026-09-12 and was cherry-picked into v26.10.0 (Current) via nodejs/node#66163 on 2026-09-22. The premise holds, and keeping toWebReadableStream is still correct since CI also runs Node 20/22/24.

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

@dinwwwh
dinwwwh merged commit 2f36999 into main Sep 25, 2026
16 of 17 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.

1 participant