feat: upgrade to undici@8#820
Conversation
undici@8 negotiates HTTP/2 by default (allowH2 now defaults to true), so urllib uses HTTP/2 with capable servers unless a custom dispatcher opts out. Raise the minimum Node.js version to 22.19.0 and test Node 22, 24, 26 in CI. Test updates for undici@8 behavior changes: - HTTP/2 headers timeout now surfaces as HeadersTimeoutError - allowH2:true multiplexes requests over a single h2 connection - close ProxyAgent instances in teardown to avoid a shutdown hang Closes #765 BREAKING CHANGE: requires Node.js >= 22.19.0 and negotiates HTTP/2 by default
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughUpgrades undici from v7 to v8 and raises the Node.js minimum to 22.19.0. Adds Changesundici@8 Upgrade and allowH2 Negotiation
Sequence Diagram(s)sequenceDiagram
participant Caller
participant urllib_request as urllib.request()
participant getDefaultHttpClient
participant HttpClient
participant undici_Dispatcher as undici Dispatcher
Caller->>urllib_request: request(url, { allowH2: false })
urllib_request->>getDefaultHttpClient: getDefaultHttpClient(rejectUnauthorized, allowH2)
getDefaultHttpClient->>getDefaultHttpClient: lookup Map key "false:false"
getDefaultHttpClient-->>urllib_request: cached or new HttpClient(allowH2: false)
urllib_request->>HttpClient: request(url, { allowH2: false })
HttpClient->>HttpClient: effectiveAllowH2 = args.allowH2 ?? `#allowH2`
HttpClient->>undici_Dispatcher: dispatch({ allowH2: false }) [skip if MockAgent]
undici_Dispatcher-->>HttpClient: HTTP/1.1 response
HttpClient-->>Caller: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #820 +/- ##
==========================================
+ Coverage 94.58% 94.91% +0.32%
==========================================
Files 10 10
Lines 739 747 +8
Branches 231 235 +4
==========================================
+ Hits 699 709 +10
+ Misses 37 35 -2
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades undici to version 8.4.1 and updates the Node engine requirement to >= 22.19.0. It also updates type definitions for AbortSignal, adjusts test assertions to align with undici@8's default HTTP/2 multiplexing and error reporting behavior, and ensures proxy agents are properly closed in tests. The review comments highlight critical issues where explicitly setting allowH2: false is ignored because the codebase only checks for truthiness when configuring the dispatcher or caching client instances. Consequently, clients fall back to the default undici@8 behavior, which enables HTTP/2 by default. Additionally, it is recommended to add assertions in the tests to verify that HTTP/2 is indeed disabled when allowH2 is set to false.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // undici@8 negotiates HTTP/2 with h2-capable servers, so all requests are | ||
| // multiplexed over a single connection instead of opening one per request. | ||
| assert.equal(httpClient.getDispatcherPoolStats()['https://registry.npmmirror.com'].connected, 1); | ||
| assert(httpClient.getDispatcherPoolStats()[_url.substring(0, _url.length - 1)].connected > 1); |
There was a problem hiding this comment.
The test should work with allowH2 = true creates httpClient1 with allowH2: false but never asserts that HTTP/2 is actually disabled for it (it only asserts response.status === 200).
Because of the bug in HttpClient's constructor and getDefaultHttpClient where allowH2: false is ignored, httpClient1 is actually still using HTTP/2 under the hood.
We should add an assertion to verify that httpClient1 indeed uses HTTP/1.1 (e.g., by checking that it does not multiplex requests or by checking the socket/protocol if available) to prevent this regression.
Cover undici@8 default behavior with local TLS servers: HTTP/1.1 fallback when the server has no HTTP/2 support, and HTTP/2 negotiation when it does.
vp config no longer tracks the vite-plus MCP server settings or skill symlink under .claude/, so remove the committed files.
commit: |
Since undici@8 defaults allowH2 to true, an explicit allowH2: false was
silently ignored and HTTP/2 was still negotiated. The HttpClient constructor
now pins a dedicated dispatcher whenever allowH2 is set (true or false), and
getDefaultHttpClient caches separate clients for the allowH2: false case so
request(url, { allowH2: false }) forces HTTP/1.1.
Addresses review feedback on #820.
|
Addressed in b6d10ab. The three comments share one root cause: undici@8 defaults
This keeps undici@8's HTTP/2 default (unset |
Keep patch coverage strict while allowing a 1% project threshold, so external-network test jitter and undici@8 protocol-path changes (an h2 error branch now reachable only via upgrade streams) don't fail CI.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2dfc9a3e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "qs": "^6.15.0", | ||
| "type-fest": "^4.41.0", | ||
| "undici": "^7.24.0", | ||
| "undici": "^8.4.1", |
There was a problem hiding this comment.
With this upgrade, omitting allowH2 now means HTTP/2 is enabled by default, but the same ClientOptions type is also used by FetchFactory.setClientOptions(): when callers pass only { allowH2: false }, src/fetch.ts still checks clientOptions?.allowH2 truthily and constructs a BaseAgent without the option, so undici v8 negotiates HTTP/2 anyway. This leaves fetch users without the documented HTTP/1.1 opt-out unless they also provide connect/lookup or a custom dispatcher.
Useful? React with 👍 / 👎.
fetch shared the same truthy-check bug: passing only { allowH2: false } left
the BaseAgent without the option, so undici@8 still negotiated HTTP/2. Pin the
dispatcher whenever allowH2 is set explicitly, matching HttpClient.
Addresses Codex review feedback on #820.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65ac925df4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Pin the protocol when allowH2 is set explicitly: `true` enables HTTP/2, | ||
| // `false` forces HTTP/1.1 instead of following undici@8's HTTP/2 default. | ||
| this.#dispatcher = new Agent({ | ||
| allowH2: clientOptions.allowH2, |
There was a problem hiding this comment.
Passing allowH2: false here makes undici v8 store the internal client under a key like https://host#http1-only; getDispatcherPoolStats() exposes those internal kClients keys as documented origins, so callers using new HttpClient({ allowH2: false }) or request(..., { allowH2: false }) can no longer look up stats by the origin URL and get undefined. Consider normalizing that suffix (or using the dispatcher origin) when building the stats map.
Useful? React with 👍 / 👎.
| if (allowH2 === false) { | ||
| if (!disallowH2HttpClient) { | ||
| disallowH2HttpClient = new HttpClient({ | ||
| allowH2, | ||
| }); |
There was a problem hiding this comment.
In applications that install a global dispatcher with setGlobalDispatcher() (for example a ProxyAgent or MockAgent), request(url, { allowH2: false }) now takes this new branch and creates urllib's own Agent, so the request no longer goes through the configured global proxy/mock; before this change the falsy option fell through to the default client/global dispatcher. Consider applying the HTTP/1-only choice without bypassing the active dispatcher, or requiring callers to pass an explicit dispatcher for this combination.
Useful? React with 👍 / 👎.
Addresses Codex review feedback on #820: - allowH2: false no longer creates urllib's own Agent (which bypassed a user-installed global ProxyAgent/dispatcher). It is now forwarded as a per-request undici option, so HTTP/1.1 is forced through the active dispatcher. Only allowH2: true still uses a dedicated agent. - getDispatcherPoolStats() normalizes undici@8's `${origin}#http1-only` pool keys back to the origin so stats remain reachable by URL.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14acd12754
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Skip per-request allowH2 forwarding when the active dispatcher is a
MockAgent: undici keys http1-only clients as `${origin}#http1-only`, which
would miss interceptors registered on the plain origin (protocol choice is
moot when mocking).
- normalizePoolStatsKey tolerates non-string client keys (MockAgent RegExp /
function origin matchers) instead of throwing.
- getDefaultHttpClient caches a distinct allowH2: false client again so
getDefaultHttpClient(undefined, false).request(url) forces HTTP/1.1 without a
per-request flag, while still using the global dispatcher.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8c1c98aac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses Codex review feedback on #820: - HttpAgent no longer bakes allowH2 into its `connect` options. undici builds the connector as `buildConnector({ allowH2, ...connect })`, so a connect-level allowH2 shadowed the top-level/per-request value; keeping it top-level lets `request(..., { allowH2: false })` force HTTP/1.1 for lookup/checkAddress clients too. - mergePoolStat coerces missing counters to 0, so undici ClientStats (Agent with connections: 1, no free/queued) no longer produce NaN in getDispatcherPoolStats().
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44ef14ac03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the plain origin (protocol negotiation is moot when mocking anyway). | ||
| const activeDispatcher = requestOptions.dispatcher ?? getGlobalDispatcher(); | ||
| if (!(activeDispatcher instanceof MockAgent)) { | ||
| requestOptions.allowH2 = allowH2; |
There was a problem hiding this comment.
When callers pass a Pool/Client as options.dispatcher, setting requestOptions.allowH2 here does not force HTTP/1.1: undici only uses the dispatch-level flag in Agent to choose a separate #http1-only client, while a Pool or Client has already built its connector from constructor options. In that scenario, request(url, { dispatcher: new Pool(origin), allowH2: false }) still negotiates HTTP/2 against an h2-capable server under undici@8 despite the new per-request option, so this combination needs special handling or should be rejected/documented as unsupported.
Useful? React with 👍 / 👎.
Clarify on the allowH2 request option that the per-request HTTP/1.1 opt-out is honored by Agent-based dispatchers (default agent, ProxyAgent) but cannot downgrade a raw Pool/Client built for HTTP/2, and is not applied to MockAgent. Addresses Codex review feedback on #820.
|
Addressed in 3d04056. Both points are inherent undici@8 constraints (the per-request
The well-supported path ( |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
Exercise getDefaultHttpClient(false, false/true/undefined) so the cached allowH2: false client for rejectUnauthorized is covered by tests.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- mergePoolStat: add a full all-counter sum case (previously only the
zero-coercion path asserted, with a partial second stat).
- getDispatcherPoolStats: drive a real collision by issuing one HTTP/2 and one
allowH2: false HTTP/1.1 request to the same origin on one dispatcher, then
assert the `${origin}` and `${origin}#http1-only` pools collapse into a single
origin entry whose connected count is the sum (2), not an overwrite.
Instead of a single request per protocol, issue several concurrent HTTP/2 and
allowH2: false HTTP/1.1 requests so both the `${origin}` and `${origin}#http1-only`
pools hold multiple connections. Verify the merged getDispatcherPoolStats() entry
equals the field-by-field sum of the two raw undici pools (observed e.g. 5 + 8 = 13
connected), proving a real sum rather than an overwrite.
Code-review cleanups (no behavior change to the public API): - Extract the duplicated getDispatcherPoolStats iteration into a shared buildPoolStats(agent) used by both HttpClient and FetchFactory; drop the now unused Pool/undici symbols imports from fetch.ts. - Hoist the `#http1-only` pool-key suffix into a constant and simplify mergePoolStat to a single summing expression. - Harden the allowH2 MockAgent skip with a constructor-name fallback so a MockAgent from a duplicate undici install is still detected. - Fix the getDefaultHttpClient comment: rejectUnauthorized + allowH2:false clients do own a dispatcher (to carry the TLS option).
…ures Quality cleanups from /simplify (no public behavior change): - getDefaultHttpClient: replace the 6 module-level singletons + nested branch ladder with one Map keyed by (rejectUnauthorized, allowH2). - Extract an isMockAgent() predicate and flatten the per-request allowH2 guard. - Drop the redundant allowH2 redeclaration on UrllibRequestOptions (inherited from RequestOptions) and the single-use AbortSignal type alias. - Tests: share a startH2EchoServer() fixture across the protocol-negotiation / pool-stats tests instead of repeating the same secure-server boilerplate.
There was a problem hiding this comment.
Pull request overview
This PR upgrades undici to v8, raises the minimum supported Node.js version to match undici’s engine requirement, and adapts urllib’s dispatcher/HTTP protocol behavior to undici@8’s default HTTP/2 negotiation (allowH2 now defaults to true).
Changes:
- Upgrade
undicito^8.4.1and bumpengines.nodeto>= 22.19.0(plus lockfile updates). - Update HttpClient/FetchFactory behavior and types to support per-request
allowH2handling and to normalize/merge pool stats with undici@8’s#http1-onlypool keys. - Refresh CI/test expectations for undici@8 (HTTP/2 multiplexing, timeout error types/messages, teardown cleanup), add Codecov config, and update CI Node matrix.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
package.json |
Bumps undici dependency and Node engine requirement. |
pnpm-lock.yaml |
Lockfile updates for undici@8 and related dependency bumps. |
.github/workflows/nodejs.yml |
Updates CI test matrix to Node 22/24/26. |
codecov.yml |
Adds Codecov thresholds (strict patch coverage, tolerant project coverage). |
src/Request.ts |
Adds/updates allowH2 option docs and tightens signal typing for undici@8. |
src/index.ts |
Refactors default HttpClient caching to include allowH2 combinations. |
src/HttpClient.ts |
Implements per-request allowH2, MockAgent handling, and normalized/merged pool stats helpers. |
src/HttpAgent.ts |
Adjusts connector option shaping to keep allowH2 top-level (to support per-request overrides). |
src/fetch.ts |
Pins allowH2 when explicitly set; reuses shared pool stats builder. |
test/options.timeout.test.ts |
Updates expected HTTP/2 timeout error type/code/message for undici@8. |
test/options.dispatcher.test.ts |
Ensures ProxyAgent instances are closed in teardown to avoid shutdown hangs. |
test/index.test.ts |
Adds regression test ensuring MockAgent interceptors aren’t bypassed by allowH2: false. |
test/HttpClient.test.ts |
Adds ALPN negotiation + pool-stats merge tests and updates HTTP/2 multiplexing expectations. |
test/fetch.test.ts |
Adds test ensuring setClientOptions({ allowH2: false }) is honored. |
.claude/settings.json |
Removes Claude MCP settings file from repo. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Keep allowH2 at the top level only. undici builds the connector as | ||
| // `buildConnector({ allowH2, ...connect })`, so an allowH2 inside `connect` | ||
| // would shadow the top-level/per-request value and defeat `allowH2: false`. | ||
| connect: { ...options.connect, lookup: lookupFunction }, |
…test fixtures From /code-review: - normalizePoolStatsKey strips only a trailing `#http1-only` suffix (endsWith) instead of the first indexOf occurrence. - getDefaultHttpClient cache key is self-descriptive (`rejectUnauthorized=...:allowH2=...`) instead of ternary-of-ternaries ints. - Share startEchoServer() so the HTTP/1.1-only test reuses the fixture too, and the fixture owns teardown (returns close()), removing hand-rolled server.close promises from every protocol test. - Add missing teardown to the `should timeout on h2` test (server + dispatcher). - Fix the merge test's inaccurate "multiplex over a single connection" comment and drop a tautological NaN assertion in the mergePoolStat unit test.
Upgrade undici from v7 to v8.
Breaking changes
>= 22.0.0).allowH2to defaulttrue, so requests to HTTP/2-capable TLS servers now use HTTP/2 instead of HTTP/1.1. PassallowH2: false(per request or per client) to force HTTP/1.1.allowH2: falseis applied per request through the active dispatcher (default agent /ProxyAgent). It cannot downgrade a rawPool/Clientpassed asdispatcher(construct those withallowH2: false), and is not applied underMockAgent.HeadersTimeoutError/UND_ERR_HEADERS_TIMEOUT(wasInformationalError/UND_ERR_INFO); code matching onerr.causemust update.getDispatcherPoolStats()normalizes undici@8's${origin}#http1-onlypool keys back to the origin, so a single origin entry may merge HTTP/2 and HTTP/1.1 pool stats.Other changes
Request.ts:signaltyped asglobalThis.AbortSignal(v8 has a strictersignaltype).Closes #765
Summary by CodeRabbit
allowH2option for per-request HTTP/2 protocol negotiation control (HTTP/2 is enabled by default).undicito^8.4.1.>= 22.19.0.