-
Notifications
You must be signed in to change notification settings - Fork 126
feat: upgrade to undici@8 #820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
a06df01
ccc6ef9
ac143d3
b6d10ab
f2dfc9a
65ac925
14acd12
e8c1c98
44ef14a
3d04056
9cf39ef
67a0c59
7a2a1a7
9424b74
d484db3
a9a0b2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # Patch coverage stays strict (new/changed lines must be covered). | ||
| # Project coverage tolerates small fluctuations from external-network tests | ||
| # and undici behaviour changes (e.g. an error branch reachable only on a | ||
| # different protocol path). | ||
| coverage: | ||
| status: | ||
| project: | ||
| default: | ||
| target: auto | ||
| threshold: 1% | ||
| patch: | ||
| default: | ||
| target: auto | ||
| threshold: 0% |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ import { createGunzip, createBrotliDecompress, gunzipSync, brotliDecompressSync | |
| import FormStream from 'formstream'; | ||
| import mime from 'mime-types'; | ||
| import qs from 'qs'; | ||
| import { request as undiciRequest, Dispatcher, Agent, getGlobalDispatcher, Pool } from 'undici'; | ||
| import { request as undiciRequest, Dispatcher, Agent, getGlobalDispatcher, MockAgent, Pool } from 'undici'; | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| import undiciSymbols from 'undici/lib/core/symbols.js'; | ||
|
|
@@ -38,7 +38,11 @@ import { parseJSON, digestAuthHeader, globalId, performanceTime, isReadable, upd | |
| type Exists<T> = T extends undefined ? never : T; | ||
| type UndiciRequestOption = Exists<Parameters<typeof undiciRequest>[1]>; | ||
| type PropertyShouldBe<T, K extends keyof T, V> = Omit<T, K> & { [P in K]: V }; | ||
| type IUndiciRequestOption = PropertyShouldBe<UndiciRequestOption, 'headers', IncomingHttpHeaders>; | ||
| // undici reads `allowH2` per dispatch (Agent picks an http1-only pool when false), | ||
| // but it is not part of the public request options type, so add it explicitly. | ||
| type IUndiciRequestOption = PropertyShouldBe<UndiciRequestOption, 'headers', IncomingHttpHeaders> & { | ||
| allowH2?: boolean; | ||
| }; | ||
|
|
||
| export const PROTO_RE: RegExp = /^https?:\/\//i; | ||
|
|
||
|
|
@@ -74,7 +78,7 @@ const debug = debuglog('urllib:HttpClient'); | |
|
|
||
| export type ClientOptions = { | ||
| defaultArgs?: RequestOptions; | ||
| /** Allow to use HTTP2 first. Default is `false` */ | ||
| /** Negotiate HTTP/2 with capable servers via ALPN. Enabled by default since undici@8; set `false` to force HTTP/1.1. */ | ||
| allowH2?: boolean; | ||
| /** Custom DNS lookup function, default is `dns.lookup`. */ | ||
| lookup?: LookupFunction; | ||
|
|
@@ -171,6 +175,32 @@ export interface PoolStat { | |
| size: number; | ||
| } | ||
|
|
||
| // undici@8 keys http1-only pools (allowH2: false) as `${origin}#http1-only`; | ||
| // expose them under their plain origin so callers can look up stats by URL. | ||
| // MockAgent may use RegExp/function origin matchers, so keys are not always strings. | ||
| export function normalizePoolStatsKey(key: unknown): string { | ||
| if (typeof key !== 'string') { | ||
| return String(key); | ||
| } | ||
| const index = key.indexOf('#http1-only'); | ||
|
fengmk2 marked this conversation as resolved.
Outdated
|
||
| return index === -1 ? key : key.slice(0, index); | ||
| } | ||
|
|
||
| // When both an HTTP/2 and an http1-only pool exist for the same origin, sum | ||
| // their stats so a single origin entry reflects every client. undici ClientStats | ||
| // (Agent with connections: 1) omit free/queued, so treat absent counters as 0. | ||
| export function mergePoolStat(existing: PoolStat | undefined, stats: Partial<PoolStat>): PoolStat { | ||
| const base = existing ?? { connected: 0, free: 0, pending: 0, queued: 0, running: 0, size: 0 }; | ||
| return { | ||
| connected: base.connected + (stats.connected ?? 0), | ||
| free: base.free + (stats.free ?? 0), | ||
| pending: base.pending + (stats.pending ?? 0), | ||
| queued: base.queued + (stats.queued ?? 0), | ||
| running: base.running + (stats.running ?? 0), | ||
| size: base.size + (stats.size ?? 0), | ||
| }; | ||
| } | ||
|
|
||
| // https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections | ||
| const RedirectStatusCodes = [ | ||
| 301, // Moved Permanently | ||
|
|
@@ -189,10 +219,14 @@ const CrossOriginSensitiveHeaders = new Set(['authorization', 'cookie', 'proxy-a | |
| export class HttpClient extends EventEmitter { | ||
| #defaultArgs?: RequestOptions; | ||
| #dispatcher?: Dispatcher; | ||
| #allowH2?: boolean; | ||
|
|
||
| constructor(clientOptions?: ClientOptions) { | ||
| super(); | ||
| this.#defaultArgs = clientOptions?.defaultArgs; | ||
| // Remember the client-level protocol preference so it can be applied per | ||
| // request (mainly for `allowH2: false`, which has no dedicated agent). | ||
| this.#allowH2 = clientOptions?.allowH2; | ||
| if (clientOptions?.lookup || clientOptions?.checkAddress) { | ||
| this.#dispatcher = new HttpAgent({ | ||
| lookup: clientOptions.lookup, | ||
|
|
@@ -206,7 +240,8 @@ export class HttpClient extends EventEmitter { | |
| allowH2: clientOptions.allowH2, | ||
| }); | ||
| } else if (clientOptions?.allowH2) { | ||
| // Support HTTP2 | ||
| // Support HTTP/2 with a dedicated agent. `allowH2: false` is handled | ||
| // per request instead, so it does not bypass the active dispatcher. | ||
| this.#dispatcher = new Agent({ | ||
| allowH2: clientOptions.allowH2, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Passing Useful? React with 👍 / 👎. |
||
| }); | ||
|
|
@@ -236,14 +271,10 @@ export class HttpClient extends EventEmitter { | |
| const stats = pool?.stats ?? pool?.dispatcher?.stats; | ||
| if (!stats) continue; | ||
|
|
||
| poolStatsMap[key] = { | ||
| connected: stats.connected, | ||
| free: stats.free, | ||
| pending: stats.pending, | ||
| queued: stats.queued, | ||
| running: stats.running, | ||
| size: stats.size, | ||
| } satisfies PoolStat; | ||
| // undici@8 keys http1-only pools (allowH2: false) as `${origin}#http1-only`, | ||
| // expose them by their origin so callers can look up stats by URL. | ||
| const origin = normalizePoolStatsKey(key); | ||
| poolStatsMap[origin] = mergePoolStat(poolStatsMap[origin], stats); | ||
| } | ||
| return poolStatsMap; | ||
| } | ||
|
|
@@ -441,6 +472,18 @@ export class HttpClient extends EventEmitter { | |
| signal: args.signal, | ||
| reset: false, | ||
| }; | ||
| const allowH2 = args.allowH2 ?? this.#allowH2; | ||
| if (typeof allowH2 === 'boolean') { | ||
| // Apply the protocol preference per request so the active dispatcher | ||
| // (global, proxy, ...) is honored instead of being bypassed by a | ||
| // dedicated HTTP/1.1-only agent. Skip it for MockAgent: it keys clients | ||
| // as `${origin}#http1-only` and would miss interceptors registered on | ||
| // the plain origin (protocol negotiation is moot when mocking anyway). | ||
| const activeDispatcher = requestOptions.dispatcher ?? getGlobalDispatcher(); | ||
| if (!(activeDispatcher instanceof MockAgent)) { | ||
| requestOptions.allowH2 = allowH2; | ||
|
fengmk2 marked this conversation as resolved.
Outdated
fengmk2 marked this conversation as resolved.
Outdated
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When callers pass a Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
| if (typeof args.highWaterMark === 'number') { | ||
| requestOptions.highWaterMark = args.highWaterMark; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this upgrade, omitting
allowH2now means HTTP/2 is enabled by default, but the sameClientOptionstype is also used byFetchFactory.setClientOptions(): when callers pass only{ allowH2: false },src/fetch.tsstill checksclientOptions?.allowH2truthily and constructs aBaseAgentwithout the option, so undici v8 negotiates HTTP/2 anyway. This leaves fetch users without the documented HTTP/1.1 opt-out unless they also provideconnect/lookupor a custom dispatcher.Useful? React with 👍 / 👎.