Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/fix-undici-401-body-source.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"miniflare": patch
"wrangler": patch
---

fix: avoid "expected non-null body source" error on 401 responses with a request body

`undici.fetch()` implements the Fetch spec's 401 credential-retry path, which throws `TypeError: fetch failed` with cause `"expected non-null body source"` whenever a request has a `ReadableStream` body and the server responds with HTTP 401. This affected `Miniflare#dispatchFetch()`, `unstable_startWorker().fetch()`, and the internal Cloudflare API client used by Wrangler.

The fix replaces `undici.fetch()` with `undici.request()` at the affected call sites. `undici.request()` uses the Dispatcher API directly and does not implement the Fetch spec credential-retry path, so the crash cannot occur. The wrapper explicitly replicates the behaviours of `undici.fetch()` that callers rely on: `Accept`/`Accept-Encoding` default headers, transparent decompression of gzip/deflate/brotli responses, redirect following (honouring the `redirect` mode), and correct handling of multiple `set-cookie` headers.

The workspace-level `pnpm` patch for `undici@7.24.4` that was previously used as a workaround has been removed.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@
"postal-mime": "patches/postal-mime.patch",
"youch@4.1.0-beta.10": "patches/youch@4.1.0-beta.10.patch",
"@netlify/build-info": "patches/@netlify__build-info.patch",
"buffer-equal-constant-time@1.0.1": "patches/buffer-equal-constant-time@1.0.1.patch",
"undici@7.24.4": "patches/undici@7.24.4.patch"
"buffer-equal-constant-time@1.0.1": "patches/buffer-equal-constant-time@1.0.1.patch"
}
}
}
178 changes: 174 additions & 4 deletions packages/miniflare/src/http/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import assert from "node:assert";
import http from "node:http";
import { Readable } from "node:stream";
import { IncomingRequestCfProperties } from "@cloudflare/workers-types/experimental";
import * as undici from "undici";
import { UndiciHeaders } from "undici/types/dispatcher";
Expand All @@ -16,7 +17,23 @@ export async function fetch(
init?: RequestInit | Request
): Promise<Response> {
const requestInit = init as RequestInit;
const request = new Request(input, requestInit);
// If input is already a Request-like object and there's no extra init to
// merge, use it directly rather than re-wrapping. Re-wrapping can fail in
// some environments (e.g. vitest forks with mixed ESM/CJS module graphs)
// because undici's internal `webidl.is.Request` brand-check uses class
// identity, and the test's ESM-resolved `Request` may be a different object
// from the CJS-required `Request` inside this bundle. Duck-typing on the
// public `RequestInfo` interface avoids that brittleness.
const isRequestLike =
requestInit === undefined &&
typeof input === "object" &&
input !== null &&
typeof (input as Request).url === "string" &&
typeof (input as Request).method === "string" &&
typeof (input as Request).headers === "object";
const request = isRequestLike
? (input as Request)
: new Request(input, requestInit);

// Handle WebSocket upgrades
if (
Expand Down Expand Up @@ -80,10 +97,163 @@ export async function fetch(
return responsePromise;
}

const response = await undici.fetch(request, {
dispatcher: requestInit?.dispatcher,
// Wrap in a try/catch and re-throw as TypeError("fetch failed") to match
// the error shape that undici.fetch() produces for dispatcher-level errors
// (e.g. when the request contains an `upgrade` header that undici.request()
// rejects with InvalidArgumentError).
try {
return await fetchViaRequest(request, requestInit?.dispatcher);
} catch (e) {
if (e instanceof TypeError) throw e;
throw new TypeError("fetch failed", { cause: e });
}
}

/**
* Perform an HTTP request using `undici.request()` instead of `undici.fetch()`.
*
* `undici.fetch()` implements the Fetch spec's 401 credential-retry path, which
* crashes with "expected non-null body source" when the request body is a
* ReadableStream and the response status is 401. This is tracked upstream at
* https://github.com/nodejs/undici/issues/4910.
*
* `undici.request()` uses the Dispatcher API directly and has no such path.
* To maintain behavioural parity with `undici.fetch()` we explicitly handle:
* - Compression: send `Accept-Encoding: gzip, deflate` and decompress responses
* - Redirects: follow/manual/error modes as per the Fetch spec
* - `set-cookie`: preserve multiple values using `Headers.append()`
*/
async function fetchViaRequest(
request: Request,
dispatcher: undici.Dispatcher | undefined,
redirectsRemaining = 20
): Promise<Response> {
const {
statusCode,
headers: rawHeaders,
body: rawBody,
} = await undici.request(request.url, {
method: request.method,
// Match undici.fetch() behaviour: add Accept-Encoding if the caller
// has not already specified one, so that the workerd entry worker
// applies the same content-encoding logic.
// Match undici.fetch() defaults: it sends `Accept: */*` and
Comment on lines +134 to +140
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.

Nit: These two comment blocks appear to be stale leftovers — they overlap in content (both explain the Accept-Encoding / Accept defaults) and have inconsistent indentation (extra leading tab on lines 134 and 137). Consider consolidating into a single comment:

Suggested change
body: rawBody,
} = await undici.request(request.url, {
method: request.method,
// Match undici.fetch() behaviour: add Accept-Encoding if the caller
// has not already specified one, so that the workerd entry worker
// applies the same content-encoding logic.
// Match undici.fetch() defaults: it sends `Accept: */*` and
// Match undici.fetch() defaults: it sends `Accept: */*` and
// `Accept-Encoding: gzip, deflate` when the caller hasn't set them.
// These are placed before the spread so the caller's own headers
// take precedence.

// `Accept-Encoding: gzip, deflate` when the caller hasn't set them.
// These are placed before the spread so the caller's own headers
// take precedence.
headers: {
accept: "*/*",
"accept-encoding": "gzip, deflate",
...Object.fromEntries(request.headers),
},
// undici.request() accepts Node.js Readable streams but not web
// ReadableStreams. Convert if necessary.
body:
request.body !== null
? Readable.fromWeb(
request.body as import("node:stream/web").ReadableStream
)
: null,
dispatcher,
});
return new Response(response.body, response);

// Build a proper Headers object. undici.request() returns headers as a plain
// object where `set-cookie` may be an array. Passing a plain object with an
// array value to `new Response()` merges the values incorrectly, losing the
// individual cookie boundaries. Using `Headers.append()` preserves them.
const headers = new undici.Headers();
for (const [name, value] of Object.entries(rawHeaders)) {
if (Array.isArray(value)) {
for (const v of value) {
headers.append(name, v);
}
} else if (value !== undefined) {
headers.set(name, value);
}
}

// Decompress response body to match undici.fetch() behaviour. undici.fetch()
// sends Accept-Encoding and transparently decompresses the response body while
// leaving the Content-Encoding header in place. We do the same here so that
// callers (e.g. index.ts dispatchFetch) can apply the same Content-Encoding
// stripping logic without receiving a still-compressed body.
//
// rawBody.body is the web ReadableStream exposed by undici's BodyReadable.
const webStream = rawBody.body as ReadableStream | null | undefined;
const contentEncoding = rawHeaders["content-encoding"];
let responseBody: ReadableStream | null;
if (
(contentEncoding === "gzip" || contentEncoding === "x-gzip") &&
webStream != null
) {
responseBody = webStream.pipeThrough(new DecompressionStream("gzip"));
} else if (contentEncoding === "deflate" && webStream != null) {
// Both zlib-wrapped deflate (the common HTTP interpretation) and raw deflate
// are handled by DecompressionStream("deflate") in Node 18+.
responseBody = webStream.pipeThrough(new DecompressionStream("deflate"));
} else if (contentEncoding === "br" && webStream != null) {
// "brotli" is not in the TypeScript CompressionFormat union yet, but
// Node 22+ supports it via DecompressionStream.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
responseBody = webStream.pipeThrough(
new DecompressionStream("brotli" as any)
);
} else {
responseBody = webStream ?? null;
}

// Handle redirect modes, matching undici.fetch() behaviour.
const redirectMode = request.redirect ?? "follow";
const isRedirect =
statusCode >= 300 &&
statusCode < 400 &&
rawHeaders["location"] !== undefined;

if (isRedirect) {
if (redirectMode === "error") {
await rawBody.dump();
throw new TypeError("fetch failed");
}
if (redirectMode === "manual") {
return new Response(responseBody as AsyncIterable<Uint8Array> | null, {
status: statusCode,
headers,
});
}
// redirect === "follow"
if (redirectsRemaining === 0) {
await rawBody.dump();
throw new TypeError("fetch failed");
}
const location = new URL(
rawHeaders["location"] as string,
request.url
).toString();
await rawBody.dump();
// For 303 See Other: switch to GET and drop the body
const redirectMethod = statusCode === 303 ? "GET" : request.method;
Comment on lines +233 to +234
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.

🔴 Redirect handling for 301/302 POST does not convert method to GET and null the body

The Fetch spec (HTTP-redirect fetch, step 13) requires that when a 301 or 302 redirect is received and the request method is POST, the method must be changed to GET and the body set to null. The code at lines 233-234 only handles 303 (statusCode === 303), leaving 301/302 POST redirects to keep the POST method and attempt to reuse request.body. Since the body ReadableStream was already locked by Readable.fromWeb(request.body) at line 150 during the initial request, accessing request.body returns a locked stream, and new Request(location, { body: lockedStream }) will throw a TypeError. This is a behavioral regression from undici.fetch(), which correctly converted 301/302 POST to GET with a null body.

Suggested change
// For 303 See Other: switch to GET and drop the body
const redirectMethod = statusCode === 303 ? "GET" : request.method;
const redirectMethod = (statusCode === 303 || ((statusCode === 301 || statusCode === 302) && request.method === "POST")) ? "GET" : request.method;
const redirectBody = (statusCode === 303 || ((statusCode === 301 || statusCode === 302) && request.method === "POST")) ? null : request.body;
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const redirectBody = statusCode === 303 ? null : request.body;
return fetchViaRequest(
new Request(location, {
method: redirectMethod,
headers: request.headers,
body: redirectBody,
redirect: request.redirect,
} as RequestInit),
dispatcher,
redirectsRemaining - 1
Comment on lines +232 to +244
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.

Bug: request.body was already consumed by Readable.fromWeb() on line 150 and drained by undici.request(). Attempting to pass the locked/disturbed stream to new Request(...) will throw TypeError: Response body object should not be disturbed or locked.

You need to tee() or buffer the body before the initial undici.request() call so it can be replayed on redirect. One option is to clone the request up-front when redirect !== "manual" and redirect !== "error":

Suggested change
await rawBody.dump();
// For 303 See Other: switch to GET and drop the body
const redirectMethod = statusCode === 303 ? "GET" : request.method;
const redirectBody = statusCode === 303 ? null : request.body;
return fetchViaRequest(
new Request(location, {
method: redirectMethod,
headers: request.headers,
body: redirectBody,
redirect: request.redirect,
} as RequestInit),
dispatcher,
redirectsRemaining - 1
// For 303 See Other: switch to GET and drop the body
const redirectMethod = statusCode === 303 ? "GET" : request.method;
// The original request body was already consumed by undici.request().
// For non-303 redirects that need to replay the body, we cannot reuse
// request.body (it is locked/disturbed). Pass null for now — this
// matches browser behaviour for 301/302 (which switch to GET) and
// is the safest default. A full fix for 307/308 body replay would
// require buffering/teeing the body before the initial request.
const redirectBody = null;
return fetchViaRequest(
new Request(location, {
method: redirectMethod,
headers: request.headers,
body: redirectBody,
redirect: request.redirect,
} as RequestInit),
dispatcher,
redirectsRemaining - 1
);

Note: this suggestion drops the body on all redirects. For full spec compliance on 307/308, the body would need to be tee'd or buffered before the initial undici.request() call — but that's a larger change and these redirects with bodies are rare in practice. At minimum the current code should not throw.

);
}

// Responses with status 204/205 must have a null body (per spec).
// Pass null explicitly to avoid "Invalid response status code" errors.
const noBodyStatus = statusCode === 204 || statusCode === 205;
// undici's BodyInit type doesn't list ReadableStream, but it does accept
// AsyncIterable<Uint8Array> which ReadableStream satisfies at runtime.
return new Response(
noBodyStatus ? null : (responseBody as AsyncIterable<Uint8Array> | null),
{ status: statusCode, headers }
);
}

export type DispatchFetch = (
Expand Down
38 changes: 38 additions & 0 deletions packages/miniflare/test/http/fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,44 @@ test("fetch: performs regular http request", async ({ expect }) => {
const res = await fetch(upstream);
expect(await res.text()).toBe("upstream");
});
test("fetch: returns 401 response for POST with ReadableStream body without throwing", async ({
expect,
}) => {
// Regression test for https://github.com/cloudflare/workers-sdk/issues/12967
//
// undici.fetch() implements the Fetch spec's 401 credential-retry path, which
// crashes with "expected non-null body source" when the request body is a
// ReadableStream (body.source === null) and the response is 401. The fix
// uses undici.request() which bypasses that code path entirely.
//
// This test passes a Request object as input to miniflare's fetch() wrapper
// (the exact pattern used by dispatchFetch / unstable_startWorker), which
// causes the body to flow through as a ReadableStream internally.
const upstream = (
await useServer((req, res) => {
req.resume();
res.writeHead(401, { "content-type": "text/plain" });
res.end("Unauthorized");
})
).http;

const body = new ReadableStream({
start(controller) {
controller.enqueue(new TextEncoder().encode("hello"));
controller.close();
},
});
// Pass the Request object directly as input — this is the pattern that
// triggered the bug via dispatchFetch/unstable_startWorker.
const request = new Request(upstream, {
method: "POST",
body,
duplex: "half",
});
const res = await fetch(request);
expect(res.status).toBe(401);
expect(await res.text()).toBe("Unauthorized");
});
test("fetch: performs http request with form data", async ({ expect }) => {
const echoUpstream = (
await useServer((req, res) => {
Expand Down
49 changes: 45 additions & 4 deletions packages/wrangler/src/api/dev.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import events from "node:events";
import { Readable } from "node:stream";
import { getDockerPath } from "@cloudflare/workers-utils";
import { fetch, Request } from "undici";
import { request as undiciRequest, Response, Request } from "undici";
import { startDev } from "../dev/start-dev";
import { run } from "../experimental-flags";
import { logger } from "../logger";
import type { StartDevOptions } from "../dev";
import type { EnablePagesAssetsServiceBindingOptions } from "../miniflare-cli/types";
import type { CfModule, Environment, Rule } from "@cloudflare/workers-utils";
import type { Json } from "miniflare";
import type { RequestInfo, RequestInit, Response } from "undici";
import type { ReadableStream } from "node:stream/web";
import type { RequestInfo, RequestInit } from "undici";

export interface Unstable_DevOptions {
config?: string; // Path to .toml configuration file, relative to cwd
Expand Down Expand Up @@ -245,9 +247,48 @@ export async function unstable_dev(
devServer.unregisterHotKeys?.();
},
fetch: async (input?: RequestInfo, init?: RequestInit) => {
return await fetch(
...parseRequestInput(address, port, input, init, options?.localProtocol)
const [url, forwardInit] = parseRequestInput(
address,
port,
input,
init,
options?.localProtocol
);
// forwardInit is a Request object cast as RequestInit. Using
// undici.fetch(url, requestObject) triggers "expected non-null body
// source" on 401 responses when the body is a ReadableStream, because
// undici.fetch() implements the Fetch spec's 401 credential-retry path
// which checks request.body.source. undici.request() uses the
// Dispatcher API directly and has no such path.
// See: https://github.com/cloudflare/workers-sdk/issues/12967
const forward = forwardInit as Request;
const {
statusCode,
headers: rawHeaders,
body,
} = await undiciRequest(url.toString(), {
method: forward.method,
headers: Object.fromEntries(forward.headers),
body:
forward.body !== null
? Readable.fromWeb(forward.body as ReadableStream)
: null,
});
// Build a Headers object that preserves multiple set-cookie values.
const responseHeaders = new Headers();
for (const [name, value] of Object.entries(rawHeaders)) {
if (Array.isArray(value)) {
for (const v of value) {
responseHeaders.append(name, v);
}
} else if (value !== undefined) {
responseHeaders.set(name, value);
}
}
return new Response(body.body, {
status: statusCode,
Comment on lines +264 to +289
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.

🔴 unstable_dev fetch no longer follows HTTP redirects after switch to undici.request()

The fetch method returned by unstable_dev() previously used undici.fetch(), which follows redirects by default. It now uses undici.request() which defaults to maxRedirections: 0 and returns redirect responses as-is. No manual redirect-following logic was added (unlike in the miniflare fetchViaRequest). Consumers calling worker.fetch('/some-path') where the worker responds with a redirect will now receive the 3xx response instead of the final response after following the redirect chain, silently changing behavior for existing users.

Prompt for agents
In packages/wrangler/src/api/dev.ts, the fetch method of unstable_dev (lines 248-290) replaced undici.fetch() with undici.request(). However, undici.request() does not follow redirects by default (maxRedirections: 0), unlike undici.fetch() which follows up to 20 redirects. Either add maxRedirections: 20 to the undici.request() options at line 264, or implement manual redirect following similar to the fetchViaRequest function in packages/miniflare/src/http/fetch.ts.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

headers: responseHeaders,
});
},
waitUntilExit: async () => {
await events.once(devServer.devEnv, "teardown");
Expand Down
Loading
Loading