fix: ensure miniflare/wrangler can source map in the same process (…

…#4719)

.changeset/odd-fans-own.md

@@ -0,0 +1,8 @@
+---
+"miniflare": patch
+"wrangler": patch
+---
+
+fix: ensure `miniflare` and `wrangler` can source map in the same process
+
+Previously, if in a `wrangler dev` session you called `console.log()` and threw an unhandled error you'd see an error like `[ERR_ASSERTION]: The expression evaluated to a falsy value`. This change ensures you can do both of these things in the same session.

fixtures/worker-app/src/index.js

@@ -17,6 +17,7 @@ export default {
 
 		const { pathname } = new URL(request.url);
 		if (pathname === "/random") return new Response(hexEncode(randomBytes(8)));
+		if (pathname === "/error") throw new Error("Oops!");
 
 		console.log(
 			request.method,

fixtures/worker-app/tests/index.test.ts

@@ -45,6 +45,16 @@ describe("'wrangler dev' correctly renders pages", () => {
 		);
 	});
 
+	it("renders pretty error after logging", async ({ expect }) => {
+		// Regression test for https://github.com/cloudflare/workers-sdk/issues/4715
+		const response = await fetch(`http://${ip}:${port}/error`);
+		const text = await response.text();
+		expect(text).toContain("Oops!");
+		expect(response.headers.get("Content-Type")).toBe(
+			"text/html;charset=utf-8"
+		);
+	});
+
 	it("uses `workerd` condition when bundling", async ({ expect }) => {
 		const response = await fetch(`http://${ip}:${port}/random`);
 		const text = await response.text();

packages/miniflare/package.json

@@ -65,7 +65,6 @@
 		"@types/http-cache-semantics": "^4.0.1",
 		"@types/node": "^18.11.9",
 		"@types/rimraf": "^3.0.2",
-		"@types/source-map-support": "^0.5.6",
 		"@types/stoppable": "^1.1.1",
 		"@types/which": "^2.0.1",
 		"@types/ws": "^8.5.3",

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

@@ -274,3 +274,5 @@ export async function handlePrettyErrorRequest(
 		headers: { "Content-Type": "text/html;charset=utf-8" },
 	});
 }
+
+export { getFreshSourceMapSupport } from "./sourcemap";

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

@@ -2,6 +2,40 @@ import assert from "assert";
 import type { Options } from "@cspotcode/source-map-support";
 import { parseStack } from "./callsite";
 
+// `source-map-support` will only modify `Error.prepareStackTrace` if this is
+// the first time `install()` has been called. This is governed by shared data
+// stored using a well-known symbol on `globalThis`. To ensure...
+//
+// a) `miniflare` and `wrangler` can have differing `install()` options
+// b) We're not affecting external user's use of this package
+// c) `Error.prepareStackTrace` is always updated on `install()`
+//
+// ...load a fresh copy, by resetting then restoring the `require` cache, and
+// overriding `Symbol.for()` to return a unique symbol.
+export function getFreshSourceMapSupport(): typeof import("@cspotcode/source-map-support") {
+	const resolvedSupportPath = require.resolve("@cspotcode/source-map-support");
+
+	const originalSymbolFor = Symbol.for;
+	const originalSupport = require.cache[resolvedSupportPath];
+	try {
+		Symbol.for = (key) => {
+			// Make sure we only override the expected symbol. If we upgrade this
+			// package, and new symbols are used, this assertion will fail in tests.
+			// We want to guard against `source-map-support/sharedData` changing to
+			// something else. If this new symbol *should* be shared across package
+			// instances, we'll need to add an
+			// `if (key === "...") return originalSymbolFor(key);` here.
+			assert.strictEqual(key, "source-map-support/sharedData");
+			return Symbol(key);
+		};
+		delete require.cache[resolvedSupportPath];
+		return require(resolvedSupportPath);
+	} finally {
+		Symbol.for = originalSymbolFor;
+		require.cache[resolvedSupportPath] = originalSupport;
+	}
+}
+
 const sourceMapInstallBaseOptions: Options = {
 	environment: "node",
 	// Don't add Node `uncaughtException` handler
@@ -30,18 +64,7 @@ let sourceMapper: SourceMapper;
 export function getSourceMapper(): SourceMapper {
 	if (sourceMapper !== undefined) return sourceMapper;
 
-	// `source-map-support` will only modify `Error.prepareStackTrace` if this is
-	// the first time `install()` has been called. This is governed by a module
-	// level variable: `errorFormatterInstalled`. To ensure we're not affecting
-	// external user's use of this package, and so `Error.prepareStackTrace` is
-	// always updated, load a fresh copy, by resetting then restoring the
-	// `require` cache.
-
-	const originalSupport = require.cache["@cspotcode/source-map-support"];
-	delete require.cache["@cspotcode/source-map-support"];
-	const support: typeof import("@cspotcode/source-map-support") = require("@cspotcode/source-map-support");
-	require.cache["@cspotcode/source-map-support"] = originalSupport;
-
+	const support = getFreshSourceMapSupport();
 	const originalPrepareStackTrace = Error.prepareStackTrace;
 	support.install(sourceMapInstallBaseOptions);
 	const prepareStackTrace = Error.prepareStackTrace;

packages/miniflare/src/plugins/index.ts

@@ -93,6 +93,7 @@ export {
 	ModuleDefinitionSchema,
 	SourceOptionsSchema,
 	ProxyClient,
+	getFreshSourceMapSupport,
 } from "./core";
 export type {
 	ModuleRuleType,

packages/wrangler/package.json

@@ -102,7 +102,6 @@
 	},
 	"dependencies": {
 		"@cloudflare/kv-asset-handler": "^0.2.0",
-		"@cspotcode/source-map-support": "0.8.1",
 		"@esbuild-plugins/node-globals-polyfill": "^0.2.3",
 		"@esbuild-plugins/node-modules-polyfill": "^0.2.2",
 		"blake3-wasm": "^2.1.5",
@@ -124,6 +123,7 @@
 		"@cloudflare/types": "^6.18.4",
 		"@cloudflare/workers-tsconfig": "workspace:*",
 		"@cloudflare/workers-types": "^4.20230914.0",
+		"@cspotcode/source-map-support": "0.8.1",
 		"@iarna/toml": "^3.0.0",
 		"@microsoft/api-extractor": "^7.28.3",
 		"@sentry/node": "^7.86.0",

packages/wrangler/scripts/deps.ts

@@ -12,8 +12,6 @@ export const EXTERNAL_DEPENDENCIES = [
 	// todo - bundle miniflare too
 	"selfsigned",
 	"source-map",
-	// CommonJS module using `module.require()` which isn't provided by `esbuild`
-	"@cspotcode/source-map-support",
 	"@esbuild-plugins/node-globals-polyfill",
 	"@esbuild-plugins/node-modules-polyfill",
 	"chokidar",

packages/wrangler/src/sourcemap.ts

@@ -1,6 +1,7 @@
 import assert from "node:assert";
 import fs from "node:fs";
 import url from "node:url";
+import { getFreshSourceMapSupport } from "miniflare";
 import type { Options } from "@cspotcode/source-map-support";
 import type Protocol from "devtools-protocol";
 
@@ -68,18 +69,9 @@ function getSourceMappingPrepareStackTrace(
 		return sourceMappingPrepareStackTrace;
 	}
 
-	// `source-map-support` will only modify `Error.prepareStackTrace` if this
-	// is the first time `install()` has been called. This is governed by a
-	// module level variable: `errorFormatterInstalled`. To ensure we're not
-	// affecting external user's use of this package, and so
-	// `Error.prepareStackTrace` is always updated, load a fresh copy, by
-	// resetting then restoring the `require` cache.
-	const originalSupport = require.cache["@cspotcode/source-map-support"];
-	delete require.cache["@cspotcode/source-map-support"];
-	// eslint-disable-next-line @typescript-eslint/consistent-type-imports,@typescript-eslint/no-var-requires
-	const support: typeof import("@cspotcode/source-map-support") = require("@cspotcode/source-map-support");
-	require.cache["@cspotcode/source-map-support"] = originalSupport;
-
+	// eslint-disable-next-line @typescript-eslint/consistent-type-imports
+	const support: typeof import("@cspotcode/source-map-support") =
+		getFreshSourceMapSupport();
 	const originalPrepareStackTrace = Error.prepareStackTrace;
 	support.install({
 		environment: "node",