Uh oh!
There was an error while loading. Please reload this page.
[world-vercel] Use undici for request HTTP 2 and automatic re-tries - #1211
Conversation
🦋 Changeset detectedLatest commit: 40493fa The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (47 failed)mongodb (1 failed):
turso (46 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Uh oh!
There was an error while loading. Please reload this page.
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Add a shared undici Agent (wrapped in RetryAgent) as the fetch dispatcher for all outgoing HTTP calls in world-vercel. This provides: - HTTP/2 multiplexing via ALPN negotiation (allowH2: true) - Connection pooling (128 connections per origin) - Automatic retry on 429/5xx with Retry-After header support Streaming write operations (writeToStream, writeToStreamMulti, closeStream) are excluded from the H2 dispatcher because undici's HTTP/2 client hangs when combined with request bodies on PUT endpoints. Read-only stream calls (readFromStream, listStreamsByRunId) safely use the dispatcher. HTTP/1.1 pipelining is left at 1 (disabled) to avoid head-of-line blocking that deadlocks the webhook respondWith mechanism. HTTP/2 multiplexing provides the same throughput benefit without this problem. Signed-off-by: Peter Wielander <mittgfu@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| test.skipIf(isLocalDeployment())( | ||
| 'readableStreamWorkflow', | ||
| { timeout: 60_000 }, | ||
| { timeout: 80_000 }, |
There was a problem hiding this comment.
:'(
need to tackle improving test perf, especially in my benchmark PR
| const response = await fetch( | ||
| new Request(url, { method: 'GET', headers }) | ||
| ); | ||
| const response = await fetch(url, { |
There was a problem hiding this comment.
should we not use undici.request / undici.pipeline for maximum performance (as per undici's README where they recommend that to using fetch
| // Observe Retry-After header if received | ||
| retryAfter: true, | ||
| // By default, we observe re-try headers, and also separately | ||
| // re-try on these status codes: 429 / 500 / 502 / 503 / 504. |
There was a problem hiding this comment.
Are these status codes retried by default? I'm not seeing any relevant configuration for them.
| method: 'PUT', | ||
| body: chunk, | ||
| headers: httpConfig.headers, | ||
| duplex: 'half', |
There was a problem hiding this comment.
can you explain why you're removing duplex: 'half'?
There was a problem hiding this comment.
There's a comment saying:
* IMPORTANT: This dispatcher must NOT be used with `duplex: 'half'`
* streaming requests — undici's H2 client hangs when combined with
* half-duplex streams. See streamer.ts for the streaming code path.
| headers, | ||
| }); | ||
| const response = await fetch(request); | ||
| const response = await fetch(request, { |
There was a problem hiding this comment.
same. shouldn't we use undici.request for max performance over fetch?
There was a problem hiding this comment.
Claude didn't get it working, and the overhead is 0.001% or something, I think it's fine. H2 is what matters
There was a problem hiding this comment.
Note that also apparently undici.request() does not follow redirects by default, but fetch() does - so we should be careful about that (we want to follow redirects to leave the option open to redirect to S3 for the refs):
Yes, fetch() follows HTTP redirects by default, even when using undici's custom dispatcher. The key distinction is:
- undici's Agent.dispatch() / Agent.request() — maxRedirections defaults to 0 (no redirect following). This is the low-level undici API.
- fetch() (the Fetch API) — redirect defaults to 'follow' per the Fetch spec (request.js:850). This is true regardless of which dispatcher is used, because redirect handling is implemented in the Fetch spec layer (lib/web/fetch/index.js:1194), not in the dispatcher layer.
Since the PR uses fetch() with dispatcher: getDispatcher(), redirects will be followed (up to 20 times per the Fetch spec, index.js:1247). The Agent's maxRedirections: 0 setting only applies to undici's own client.request() / client.dispatch() APIs, not to fetch().
| "@workflow/errors": "workspace:*", | ||
| "@workflow/world": "workspace:*", | ||
| "cbor-x": "1.6.0", | ||
| "undici": "6.22.0", |
There was a problem hiding this comment.
can we use undici@7? I know there were some issues when I tried to bump it but maybe coudl spend some cycles to just upgrade to 7?
also we should probably put undici in catalog since local world also uses it and we should pin versions across all undici uage in our monorepo
TooTallNate
left a comment
There was a problem hiding this comment.
Solid infrastructure improvement. The HTTP/2 multiplexing and automatic retry via undici's RetryAgent are good wins. The streaming exclusion from H2 is well-documented and the reasoning is sound. A few observations inline.
| ); | ||
| } | ||
| return _dispatcher; | ||
| } |
There was a problem hiding this comment.
The module-level singleton means the dispatcher lives for the entire process lifetime with no way to shut it down. In serverless / short-lived processes this is fine, but if this code ever runs in a long-lived dev server or test suite, the 128 connections and keep-alive timers could leak.
Consider adding a closeDispatcher() export (or making it configurable via APIConfig) for test teardown. Not blocking, but worth noting.
| // By default, we observe re-try headers, and also separately | ||
| // re-try on these status codes: 429 / 500 / 502 / 503 / 504. | ||
| // TODO: We might want to let 429s pass through, so that we can do | ||
| // runtime retry-after handling through the queue. |
There was a problem hiding this comment.
The TODO about letting 429s pass through is worth tracking. With RetryAgent retrying 429s up to 5 times (the default maxRetries), and makeRequest in utils.ts also parsing Retry-After headers and throwing WorkflowAPIError with retryAfter for the queue system to handle — there's now a layered retry: undici retries 5 times silently, and only if all 5 fail does the application-level retry kick in.
This is probably fine (belt and suspenders), but the comment on utils.ts:306 ("RetryAgent handles most 429 retries automatically, but this catches the case where retries are exhausted") correctly documents it. Just want to flag that the total retry budget is now 5 * (undici retries) + queue-level retries, which could delay error surfacing.
| headers: httpConfig.headers, | ||
| } | ||
| ); | ||
| await response.text(); |
There was a problem hiding this comment.
Good addition — await response.text() ensures the response body is fully consumed, preventing connection leaks with HTTP/1.1 keep-alive. Without this, unconsumed bodies can hold connections open.
One thing: if the server returns an error status (e.g. 500), this silently discards the error body. The write methods don't check response.ok. Was this intentional? If these endpoints can fail, you might want at least a status check before the early return.
| // streaming calls. undici's experimental HTTP/2 client hangs on both | ||
| // streaming writes (PUT with body) and streaming reads (long-lived GET | ||
| // responses). Standard request/response calls (makeRequest, encryption, | ||
| // refs) work fine with H2. |
There was a problem hiding this comment.
The comment is clear and well-placed. Since this is a critical constraint, consider also adding a brief note about why the streaming calls don't use the dispatcher at all (not even for retry) — i.e., streaming writes are fire-and-forget with Uint8Array bodies (not ReadableStream), so duplex: 'half' is no longer needed, but the H2 dispatcher is still excluded because the retry behavior could cause duplicate stream writes.
| const response = await fetch(request); | ||
| const response = await fetch(request, { | ||
| dispatcher: getDispatcher(), | ||
| } as RequestInit); |
There was a problem hiding this comment.
Minor: fetch(request, { dispatcher } as RequestInit) — passing a Request object as the first argument and options as the second is a valid pattern, but note that undici's dispatcher option is read from the second argument. If Request already has a body, some implementations may not respect the dispatcher from the second arg. This works in practice with Node's built-in fetch (which is undici under the hood), but it's a subtle API surface. Just flagging for awareness.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
VaguelySerious
commented
Feb 27, 2026
Updated to undici 7 and addressed feedback, waiting for CI |
Add a shared undici Agent (wrapped in RetryAgent) as the fetch dispatcher for all outgoing HTTP calls in world-vercel. Upgrade undici to v7 and share the version via pnpm catalog between world-vercel and world-local. - HTTP/2 multiplexing via ALPN negotiation (allowH2: true) - Connection pooling (128 connections per origin) - Automatic retry on 429/5xx with Retry-After header support Streaming calls in streamer.ts are excluded from the H2 dispatcher because undici's HTTP/2 client hangs on both streaming writes and long-lived streaming reads. Signed-off-by: Peter Wielander <mittgfu@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
the same Agent