Remove legacy handler wrappers - #4786
Merged
Merged
Conversation
Member
Author
|
I used AI for this, so review with caution |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #4786 +/- ##
=======================================
Coverage 93.25% 93.25%
=======================================
Files 109 107 -2
Lines 34001 34031 +30
=======================================
+ Hits 31708 31736 +28
- Misses 2293 2295 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ronag
approved these changes
Jan 31, 2026
This was referenced Feb 2, 2026
BREAKING CHANGE: Node.js 20 is no longer supported. Minimum Node.js version is now 22. Changes: - Update engines field in package.json to >=22.0.0 - Remove Node 20 from CI test matrix - Add Node 22 to WASM SIMD disabled test matrix - Simplify cache-interceptor test to always use sqlite - Update @types/node to ^22.0.0
metcoder95
approved these changes
Feb 3, 2026
Contributor
|
This should also fix #4389. |
Contributor
|
Let me know if it would be helpful to test the |
Member
Author
|
I'll ship an alpha soon. |
Closed
1 task
1 task
This was referenced Aug 15, 2026
sethbacon
added a commit
to sethbacon/terraform-drift-report
that referenced
this pull request
Aug 15, 2026
#60) Dependabot offered undici 8.10.0 (the first offer since the duplicated override was fixed in #52). It does not work here, and the failure is a runtime one that only the real-handshake tests catch. This action hands a userland undici Agent/ProxyAgent to Node's BUILT-IN global fetch as init.dispatcher, so two copies of undici meet on every request that configures TLS trust or an egress proxy: ours, and the one compiled into the runner's Node. They have to agree on the dispatcher handler interface. undici 8.0.0 deleted the wrap-handler shim that bridged the legacy interface (nodejs/undici#4786), so assertRequestHandler now hard-requires onRequestStart/onResponseStart/onResponseData/onResponseEnd. Node 24 bundles undici 7.21.0, whose fetch still builds the LEGACY handler, hands it to our dispatcher, and undici 8 rejects it: InvalidArgumentError: invalid onRequestStart method (UND_ERR_INVALID_ARG) Every proxied callback and every private-CA callback fails. On 8.10.0 that is 6 unit tests and 21 of 56 dist checks; the plain public-CA, no-proxy path still works, which is exactly why this needed the positive controls rather than the release notes. undici 7 accepts both handler shapes, so 7.29.0 is a drop-in: the Agent and ProxyAgent constructors, `connect: { ca }`, `requestTls: { ca }` and proxy URI credentials are all unchanged from 6.x. Full suite green in both source and dist. The dependabot ignore records the rule rather than the symptom — the userland undici major must not get ahead of the major the runner's Node has built in — and says how to check when it can be lifted. Bundle grows 445 KB -> 554 KB (index.js); the metafile attributes 108,747 of the 108,726-byte delta to undici itself, which is simply a bigger library in 7.x.
sethbacon
added a commit
to sethbacon/terraform-module-publish
that referenced
this pull request
Aug 15, 2026
#63) Dependabot offered undici 8.10.0 (the first offer since the duplicated override was fixed). It does not work here, and the failure is a runtime one that only the real-handshake tests catch. This action hands a userland undici Agent/ProxyAgent to Node's BUILT-IN global fetch as init.dispatcher, so two copies of undici meet on every request that configures TLS trust or an egress proxy: ours, and the one compiled into the runner's Node. They have to agree on the dispatcher handler interface. undici 8.0.0 deleted the wrap-handler shim that bridged the legacy interface (nodejs/undici#4786), so assertRequestHandler now hard-requires onRequestStart/onResponseStart/onResponseData/onResponseEnd. Node 24 bundles undici 7.21.0, whose fetch still builds the LEGACY handler, hands it to our dispatcher, and undici 8 rejects it: InvalidArgumentError: invalid onRequestStart method (UND_ERR_INVALID_ARG) Every proxied registry call and every private-CA registry call fails. On 8.10.0 that is 6 unit tests; the plain public-CA, no-proxy path still works, which is exactly why this needed the positive controls rather than the release notes. undici 7 accepts both handler shapes, so 7.29.0 is a drop-in: the Agent and ProxyAgent constructors, `connect: { ca }`, `requestTls: { ca }` and proxy URI credentials are all unchanged from 6.x. Full suite green in both source (235 tests) and dist (92 checks). The dependabot ignore records the rule rather than the symptom — the userland undici major must not get ahead of the major the runner's Node has built in — and says how to check when it can be lifted. Bundle grows 450 KB -> 558 KB; the whole 108,740-byte delta is undici itself, which is simply a bigger library in 7.x.
mcollina
pushed a commit
that referenced
this pull request
Aug 31, 2026
/xhr was dropped from the test:wpt filter in #4786 without mention in the PR, which removed WPT FormData coverage: that suite lives under /xhr/formdata rather than /fetch. Restore the filter and record the xhr expectation entries for test files added to WPT since the removal. Also drop /serviceWorkers from the filter: the WPT directory is service-workers, so the argument has never matched any test.
This was referenced Sep 27, 2026
mgcronin
added a commit
to openzigs/metis
that referenced
this pull request
Sep 27, 2026
…314) * fix: [Issue #308] upgrade undici to 8 and wrap every dispatcher for the built-in fetch undici 8 removed its legacy-handler shim (nodejs/undici#4786). Node 22's built-in fetch still hands dispatchers a legacy handler, so every undici 8 Agent/ProxyAgent passed as `dispatcher` to globalThis.fetch failed with "invalid onRequestStart method". CI caught this only in the local-provider transport. The pinned-SSRF, webhook, importer and proxy sites broke the same way, but their tests use fake dispatchers. All dispatcher construction now goes through server/src/lib/net/builtin-fetch-dispatcher.ts, which wraps each dispatcher in Dispatcher1Wrapper as the v7->v8 migration guide says. The wrapper also keeps HTTP/1.1, where v7 already was. A source scan fails if a dispatcher is built anywhere else. New real-loopback tests cover the headers/body timeouts, abort, streaming, the FIFO limiter and the proxies through the built-in fetch. The Node floor rises to 22.19.0 (undici 8 engines), and the dev compose image moves from Node 20 to Node 22. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011jaVZMaMEggfWYuKsCKudr * test: [Issue #308] test forward proxies relay to one fixed destination CodeQL flagged the loopback test proxies as request forgery, because they forwarded to the request-line URL. They now accept only the one test target and relay to a fixed host and port. The tests assert the exact absolute-form request line, so a request that bypasses the proxy fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011jaVZMaMEggfWYuKsCKudr --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing