fix: include request url and headers in pretty error page (#4448)

This change ensures Miniflare's pretty error page includes the URL and
headers of the incoming request, rather than Miniflare's internal
request from `workerd` to the loopback server.

The request URL has been incorrect since the pretty error page was
introduced into version 3 (cloudflare/miniflare#436). The request
headers have been incorrect since (cloudflare/miniflare#681). This
change also adds some tests to prevent this regressing again.

.changeset/tiny-sloths-kneel.md

@@ -0,0 +1,7 @@
+---
+"miniflare": patch
+---
+
+fix: include request url and headers in pretty error page
+
+This change ensures Miniflare's pretty error page includes the URL and headers of the incoming request, rather than Miniflare's internal request for the page.

packages/miniflare/src/plugins/core/errors/index.ts

@@ -259,7 +259,7 @@ export async function handlePrettyErrorRequest(
 	// `cause` is usually more useful than the error itself, display that instead
 	// TODO(someday): would be nice if we could display both
 	const youch = new Youch(error.cause ?? error, {
-		url: request.url,
+		url: request.cf?.prettyErrorOriginalUrl ?? request.url,
 		method: request.method,
 		headers: Object.fromEntries(request.headers),
 	});

packages/miniflare/src/workers/core/entry.worker.ts

@@ -82,19 +82,13 @@ function maybePrettifyError(request: Request, response: Response, env: Env) {
 		return response;
 	}
 
-	// Forward `Accept` and `User-Agent` headers if defined
-	const accept = request.headers.get("Accept");
-	const userAgent = request.headers.get("User-Agent");
-	const headers = new Headers();
-	if (accept !== null) headers.set("Accept", accept);
-	if (userAgent !== null) headers.set("User-Agent", userAgent);
-
 	return env[CoreBindings.SERVICE_LOOPBACK].fetch(
 		"http://localhost/core/error",
 		{
 			method: "POST",
-			headers,
+			headers: request.headers,
 			body: response.body,
+			cf: { prettyErrorOriginalUrl: request.url },
 		}
 	);
 }

packages/miniflare/test/plugins/core/errors/index.spec.ts

@@ -5,7 +5,7 @@ import { fileURLToPath } from "url";
 import test from "ava";
 import Protocol from "devtools-protocol";
 import esbuild from "esbuild";
-import { DeferredPromise, Miniflare } from "miniflare";
+import { DeferredPromise, Log, LogLevel, Miniflare, fetch } from "miniflare";
 import type { RawSourceMap } from "source-map";
 import NodeWebSocket from "ws";
 import { escapeRegexp, useTmp } from "../../../test-shared";
@@ -268,3 +268,70 @@ async function getSources(inspectorBaseURL: URL, serviceName: string) {
 		})
 		.sort();
 }
+
+// TODO(soon): just use `NoOpLog` when `Log#error()` no longer throws
+class SafeLog extends Log {
+	error(message: Error) {
+		this.logWithLevel(LogLevel.ERROR, String(message));
+	}
+}
+
+test("responds with pretty error page", async (t) => {
+	const mf = new Miniflare({
+		log: new SafeLog(LogLevel.NONE),
+		modules: true,
+		script: `
+		function reduceError(e) {
+			return {
+				name: e?.name,
+				message: e?.message ?? String(e),
+				stack: e?.stack,
+			};
+		}
+		export default {
+			async fetch() {
+				const error = reduceError(new Error("Unusual oops!"));
+				return Response.json(error, {
+					status: 500,
+					headers: { "MF-Experimental-Error-Stack": "true" },
+				});
+			}
+		}`,
+	});
+	t.teardown(() => mf.dispose());
+	const url = new URL("/some-unusual-path", await mf.ready);
+
+	// Check `fetch()` returns pretty-error page...
+	let res = await fetch(url, {
+		method: "POST",
+		headers: { "X-Unusual-Key": "some-unusual-value" },
+	});
+	t.is(res.status, 500);
+	t.regex(res.headers.get("Content-Type") ?? "", /^text\/html/);
+	const text = await res.text();
+	// ...including error, request method, URL and headers
+	t.regex(text, /Unusual oops!/);
+	t.regex(text, /Method.+POST/s);
+	t.regex(text, /URI.+some-unusual-path/s);
+	t.regex(text, /X-Unusual-Key.+some-unusual-value/is);
+
+	// Check `fetch()` accepting HTML returns pretty-error page
+	res = await fetch(url, { headers: { Accept: "text/html" } });
+	t.is(res.status, 500);
+	t.regex(res.headers.get("Content-Type") ?? "", /^text\/html/);
+
+	// Check `fetch()` accepting text doesn't return pretty-error page
+	res = await fetch(url, { headers: { Accept: "text/plain" } });
+	t.is(res.status, 500);
+	t.regex(res.headers.get("Content-Type") ?? "", /^text\/plain/);
+	t.regex(await res.text(), /Unusual oops!/);
+
+	// Check `fetch()` as `curl` doesn't return pretty-error page
+	res = await fetch(url, { headers: { "User-Agent": "curl/0.0.0" } });
+	t.is(res.status, 500);
+	t.regex(res.headers.get("Content-Type") ?? "", /^text\/plain/);
+	t.regex(await res.text(), /Unusual oops!/);
+
+	// Check `dispatchFetch()` propagates exception
+	await t.throwsAsync(mf.dispatchFetch(url), { message: "Unusual oops!" });
+});