diff --git a/.changeset/brave-fireants-think.md b/.changeset/brave-fireants-think.md new file mode 100644 index 000000000000..fe86120f53da --- /dev/null +++ b/.changeset/brave-fireants-think.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Enable pretty source-mapped error pages when using `--experimental-local` diff --git a/packages/wrangler/src/bundle.ts b/packages/wrangler/src/bundle.ts index 0224f5c0be1c..d5a61172bb8a 100644 --- a/packages/wrangler/src/bundle.ts +++ b/packages/wrangler/src/bundle.ts @@ -28,6 +28,19 @@ type StaticAssetsConfig = }) | undefined; +/** + * When applying the middleware facade for service workers, we need to inject + * some code at the top of the final output bundle. Applying an inject too early + * will allow esbuild to reorder the code. Additionally, we need to make sure + * user code is bundled in the final esbuild step with `watch` correctly + * configured, so code changes are detected. + * + * This type is used as the return type for the `MiddlewareFn` type representing + * a facade-applying function. Returned injects should be injected with the + * final esbuild step. + */ +type EntryWithInject = Entry & { inject?: string[] }; + /** * RegExp matching against esbuild's error text when it is unable to resolve * a Node built-in module. If we detect this when node_compat is disabled, @@ -97,6 +110,7 @@ export async function bundleWorker( targetConsumer: "dev" | "publish"; local: boolean; testScheduled?: boolean; + experimentalLocal?: boolean; inject?: string[]; loader?: Record; sourcemap?: esbuild.CommonOptions["sourcemap"]; @@ -124,6 +138,7 @@ export async function bundleWorker( firstPartyWorkerDevFacade, targetConsumer, testScheduled, + experimentalLocal, inject: injectOption, loader, sourcemap, @@ -197,8 +212,29 @@ export async function bundleWorker( path: "templates/middleware/middleware-scheduled.ts", }); } + if (experimentalLocal) { + // In Miniflare 3, we bind the user's worker as a service binding in a + // special entry worker that handles things like injecting `Request.cf`, + // live-reload, and the pretty-error page. + // + // Unfortunately, due to a bug in `workerd`, errors thrown asynchronously by + // native APIs don't have `stack`s. This means Miniflare can't extract the + // `stack` trace from dispatching to the user worker service binding by + // `try/catch`. + // + // As a stop-gap solution, if the `MF-Experimental-Error-Stack` header is + // truthy on responses, the body will be interpreted as a JSON-error of the + // form `{ message?: string, name?: string, stack?: string }`. + // + // This middleware wraps the user's worker in a `try/catch`, and rewrites + // errors in this format so a pretty-error page can be shown. + middlewareToLoad.push({ + path: "templates/middleware/middleware-miniflare3-json-error.ts", + dev: true, + }); + } - type MiddlewareFn = (arg0: Entry) => Promise; + type MiddlewareFn = (currentEntry: Entry) => Promise; const middleware: (false | undefined | MiddlewareFn)[] = [ // serve static assets serveAssetsFromWorker && @@ -259,23 +295,22 @@ export async function bundleWorker( (m) => (targetConsumer === "dev" && m.dev !== false) || (m.publish && targetConsumer === "publish") - ), - moduleCollector.plugin + ) ); }), ].filter(Boolean); - let inputEntry = entry; + const inject: string[] = injectOption ?? []; + if (checkFetch) inject.push(checkedFetchFileToInject); + let inputEntry: EntryWithInject = entry; for (const middlewareFn of middleware as MiddlewareFn[]) { inputEntry = await middlewareFn(inputEntry); + if (inputEntry.inject !== undefined) inject.push(...inputEntry.inject); } // At this point, inputEntry points to the entry point we want to build. - const inject: string[] = injectOption ?? []; - if (checkFetch) inject.push(checkedFetchFileToInject); - const buildOptions: esbuild.BuildOptions & { metafile: true } = { entryPoints: [inputEntry.file], bundle: true, @@ -315,11 +350,7 @@ export async function bundleWorker( ...(loader || {}), }, plugins: [ - // We run the moduleCollector plugin for service workers as part of the middleware loader - // so we only run here for modules or with no middleware to load - ...(entry.format === "modules" || middlewareToLoad.length === 0 - ? [moduleCollector.plugin] - : []), + moduleCollector.plugin, ...(nodeCompat ? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()] : []), @@ -456,14 +487,16 @@ interface MiddlewareLoader { async function applyMiddlewareLoaderFacade( entry: Entry, tmpDirPath: string, - middleware: MiddlewareLoader[], // a list of paths to middleware files - moduleCollectorPlugin: esbuild.Plugin -): Promise { + middleware: MiddlewareLoader[] // a list of paths to middleware files +): Promise { // Firstly we need to insert the middleware array into the project, // and then we load the middleware - this insertion and loading is // different for each format. - // STEP 1: Insert the middleware + // Make sure we resolve all files relative to the actual temporary directory, + // otherwise we'll have issues with source maps + tmpDirPath = fs.realpathSync(tmpDirPath); + const targetPathInsertion = path.join( tmpDirPath, "middleware-insertion.entry.js" @@ -506,7 +539,7 @@ async function applyMiddlewareLoaderFacade( ); await esbuild.build({ - entryPoints: [path.resolve(getBasePath(), dynamicFacadePath)], + entryPoints: [dynamicFacadePath], bundle: true, sourcemap: true, format: "esm", @@ -523,98 +556,80 @@ async function applyMiddlewareLoaderFacade( ], outfile: targetPathInsertion, }); - } else { - // We handle service workers slightly differently as we have to overwrite - // the event listeners and reimplement them + let targetPathLoader = path.join(tmpDirPath, path.basename(entry.file)); + if (path.extname(entry.file) === "") targetPathLoader += ".js"; + const loaderPath = path.resolve( + getBasePath(), + "templates/middleware/loader-modules.ts" + ); await esbuild.build({ - entryPoints: [entry.file], + entryPoints: [loaderPath], bundle: true, sourcemap: true, - define: { - "process.env.NODE_ENV": `"${process.env["NODE_ENV" + ""]}"`, - }, format: "esm", - outfile: targetPathInsertion, - plugins: [moduleCollectorPlugin], + plugins: [ + esbuildAliasExternalPlugin({ + __ENTRY_POINT__: targetPathInsertion, + "./common": path.resolve( + getBasePath(), + "templates/middleware/common.ts" + ), + }), + ], + outfile: targetPathLoader, }); - + return { + ...entry, + file: targetPathLoader, + }; + } else { const imports = middlewareIdentifiers - .map( - (m, i) => - `import ${m} from "${toUrlPath( - path.resolve(getBasePath(), middleware[i].path) - )}";` - ) + .map((m) => `import ${m} from "${m}";`) .join("\n"); - - // We add the new modules with imports and then register using the - // addMiddleware function (which gets rewritten in the next build step) - - // We choose to run middleware inserted in wrangler before user inserted - // middleware in the stack - // To do this, we either need to execute the addMiddleware function first - // before any user middleware, or use a separate handling function. - // We choose to do the latter as to prepend, we would have to load the entire - // script into memory as a prepend function doesn't exist or work in the same - // way that an append function does. - - fs.copyFileSync(targetPathInsertion, dynamicFacadePath); - fs.appendFileSync( - dynamicFacadePath, - ` + const contents = `import { __facade_registerInternal__ } from "__LOADER__"; ${imports} - addMiddlewareInternal([${middlewareIdentifiers.join(",")}]) - ` - ); - } - - // STEP 2: Load the middleware - // We want to get the filename of the orginal entry point - let targetPathLoader = path.join(tmpDirPath, path.basename(entry.file)); - if (path.extname(entry.file) === "") targetPathLoader += ".js"; - - const loaderPath = - entry.format === "modules" - ? path.resolve(getBasePath(), "templates/middleware/loader-modules.ts") - : dynamicFacadePath; + __facade_registerInternal__([${middlewareIdentifiers.join(",")}]);`; + fs.writeFileSync(dynamicFacadePath, contents); - await esbuild.build({ - entryPoints: [loaderPath], - bundle: true, - sourcemap: true, - format: "esm", - ...(entry.format === "service-worker" - ? { - inject: [ - path.resolve(getBasePath(), "templates/middleware/loader-sw.ts"), - ], - define: { - addEventListener: "__facade_addEventListener__", - removeEventListener: "__facade_removeEventListener__", - dispatchEvent: "__facade_dispatchEvent__", - addMiddleware: "__facade_register__", - addMiddlewareInternal: "__facade_registerInternal__", - }, - } - : { - plugins: [ - esbuildAliasExternalPlugin({ - __ENTRY_POINT__: targetPathInsertion, - "./common": path.resolve( + await esbuild.build({ + entryPoints: [dynamicFacadePath], + bundle: true, + sourcemap: true, + format: "iife", + plugins: [ + { + name: "dynamic-facade-imports", + setup(build) { + build.onResolve({ filter: /^__LOADER__$/ }, () => { + const loaderPath = path.resolve( getBasePath(), - "templates/middleware/common.ts" - ), - }), - ], - }), - outfile: targetPathLoader, - }); - - return { - ...entry, - file: targetPathLoader, - }; + "templates/middleware/loader-sw.ts" + ); + return { path: loaderPath }; + }); + const middlewareFilter = /^__MIDDLEWARE_(\d+)__$/; + build.onResolve({ filter: middlewareFilter }, (args) => { + const match = middlewareFilter.exec(args.path); + assert(match !== null); + const middlewareIndex = parseInt(match[1]); + return { + path: path.resolve( + getBasePath(), + middleware[middlewareIndex].path + ), + }; + }); + }, + }, + ], + outfile: targetPathInsertion, + }); + return { + ...entry, + inject: [targetPathInsertion], + }; + } } /** diff --git a/packages/wrangler/src/dev/dev.tsx b/packages/wrangler/src/dev/dev.tsx index a407a48c705c..fe508e25121b 100644 --- a/packages/wrangler/src/dev/dev.tsx +++ b/packages/wrangler/src/dev/dev.tsx @@ -288,6 +288,7 @@ function DevSession(props: DevSessionProps) { // Enable the bundling to know whether we are using dev or publish targetConsumer: "dev", testScheduled: props.testScheduled ?? false, + experimentalLocal: props.experimentalLocal, }); // TODO(queues) support remote wrangler dev diff --git a/packages/wrangler/src/dev/local.tsx b/packages/wrangler/src/dev/local.tsx index f50db229f7d5..cd26a8507e29 100644 --- a/packages/wrangler/src/dev/local.tsx +++ b/packages/wrangler/src/dev/local.tsx @@ -839,17 +839,19 @@ export async function transformMf2OptionsToMf3Options({ const root = path.dirname(bundle.path); assert.strictEqual(bundle.type, "esm"); + // Required for source mapped paths to resolve correctly + options.modulesRoot = root; options.modules = [ // Entrypoint { type: "ESModule", - path: path.relative(root, bundle.path), + path: bundle.path, contents: await readFile(bundle.path, "utf-8"), }, // Misc (WebAssembly, etc, ...) ...bundle.modules.map((module) => ({ type: ModuleTypeToRuleType[module.type ?? "esm"], - path: module.name, + path: path.resolve(root, module.name), contents: module.content, })), ]; diff --git a/packages/wrangler/src/dev/start-server.ts b/packages/wrangler/src/dev/start-server.ts index a7c108da8ca1..6b1d87c04b57 100644 --- a/packages/wrangler/src/dev/start-server.ts +++ b/packages/wrangler/src/dev/start-server.ts @@ -100,6 +100,7 @@ export async function startDevServer( firstPartyWorkerDevFacade: props.firstPartyWorker, testScheduled: props.testScheduled, local: props.local, + experimentalLocal: props.experimentalLocal, }); if (props.local) { @@ -208,6 +209,7 @@ async function runEsbuild({ firstPartyWorkerDevFacade, testScheduled, local, + experimentalLocal, }: { entry: Entry; destination: string | undefined; @@ -227,6 +229,7 @@ async function runEsbuild({ firstPartyWorkerDevFacade: boolean | undefined; testScheduled?: boolean; local: boolean; + experimentalLocal: boolean | undefined; }): Promise { if (!destination) return; @@ -265,8 +268,9 @@ async function runEsbuild({ services, firstPartyWorkerDevFacade, targetConsumer: "dev", // We are starting a dev server - local, testScheduled, + local, + experimentalLocal, }); return { diff --git a/packages/wrangler/src/dev/use-esbuild.ts b/packages/wrangler/src/dev/use-esbuild.ts index d07867f8ebdb..8b782bb1a0bc 100644 --- a/packages/wrangler/src/dev/use-esbuild.ts +++ b/packages/wrangler/src/dev/use-esbuild.ts @@ -41,6 +41,7 @@ export function useEsbuild({ local, targetConsumer, testScheduled, + experimentalLocal, }: { entry: Entry; destination: string | undefined; @@ -62,6 +63,7 @@ export function useEsbuild({ local: boolean; targetConsumer: "dev" | "publish"; testScheduled: boolean; + experimentalLocal: boolean | undefined; }): EsbuildBundle | undefined { const [bundle, setBundle] = useState(); const { exit } = useApp(); @@ -134,6 +136,7 @@ export function useEsbuild({ local, targetConsumer, testScheduled, + experimentalLocal, }); // Capture the `stop()` method to use as the `useEffect()` destructor. @@ -195,6 +198,7 @@ export function useEsbuild({ local, targetConsumer, testScheduled, + experimentalLocal, ]); return bundle; } diff --git a/packages/wrangler/src/pages/functions/buildPlugin.ts b/packages/wrangler/src/pages/functions/buildPlugin.ts index dd3989202611..76bdf6709546 100644 --- a/packages/wrangler/src/pages/functions/buildPlugin.ts +++ b/packages/wrangler/src/pages/functions/buildPlugin.ts @@ -101,6 +101,7 @@ export function buildPlugin({ checkFetch: local, targetConsumer: local ? "dev" : "publish", local, + experimentalLocal: false, } ); } diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts index 44813f4c3930..5501b5ad6df9 100644 --- a/packages/wrangler/src/pages/functions/buildWorker.ts +++ b/packages/wrangler/src/pages/functions/buildWorker.ts @@ -160,6 +160,7 @@ export function buildWorker({ checkFetch: local, targetConsumer: local ? "dev" : "publish", local, + experimentalLocal: false, } ); } diff --git a/packages/wrangler/src/publish/publish.ts b/packages/wrangler/src/publish/publish.ts index e648b00db5a5..176489f3b2d3 100644 --- a/packages/wrangler/src/publish/publish.ts +++ b/packages/wrangler/src/publish/publish.ts @@ -487,6 +487,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m // This could potentially cause issues as we no longer have identical behaviour between dev and publish? targetConsumer: "publish", local: false, + experimentalLocal: false, } ); diff --git a/packages/wrangler/templates/middleware/loader-sw.ts b/packages/wrangler/templates/middleware/loader-sw.ts index 32e924ed7869..061a62be0827 100644 --- a/packages/wrangler/templates/middleware/loader-sw.ts +++ b/packages/wrangler/templates/middleware/loader-sw.ts @@ -1,8 +1,15 @@ -import { Awaitable, Dispatcher, Middleware, __facade_invoke__ } from "./common"; -export { __facade_register__, __facade_registerInternal__ } from "./common"; - -// Miniflare's `EventTarget` follows the spec and doesn't allow exceptions to -// be caught by `dispatchEvent`. Instead it has a custom`ThrowingEventTarget` +import { + Awaitable, + Dispatcher, + Middleware, + __facade_invoke__, + __facade_register__, + __facade_registerInternal__, +} from "./common"; +export { __facade_register__, __facade_registerInternal__ }; + +// Miniflare 2's `EventTarget` follows the spec and doesn't allow exceptions to +// be caught by `dispatchEvent`. Instead it has a custom `ThrowingEventTarget` // class that rethrows errors from event listeners in `dispatchEvent`. // We'd like errors to be propagated to the top-level `addEventListener`, so // we'd like to use `ThrowingEventTarget`. Unfortunately, `ThrowingEventTarget` @@ -15,45 +22,52 @@ if ((globalThis as any).MINIFLARE) { __FACADE_EVENT_TARGET__ = new EventTarget(); } -declare global { - var __facade_addEventListener__: ( - type: string, - listener: EventListenerOrEventListenerObject, - options?: EventTargetAddEventListenerOptions | boolean - ) => void; - var __facade_removeEventListener__: ( - type: string, - listener: EventListenerOrEventListenerObject, - options?: EventTargetEventListenerOptions | boolean - ) => void; - var __facade_dispatchEvent__: (event: Event) => void; -} - -function __facade_isSpecialEvent__(type: string) { +function __facade_isSpecialEvent__( + type: string +): type is "fetch" | "scheduled" { return type === "fetch" || type === "scheduled"; } -globalThis.__facade_addEventListener__ = function (type, listener, options) { +const __facade__originalAddEventListener__ = globalThis.addEventListener; +const __facade__originalRemoveEventListener__ = globalThis.removeEventListener; +const __facade__originalDispatchEvent__ = globalThis.dispatchEvent; + +globalThis.addEventListener = function (type, listener, options) { if (__facade_isSpecialEvent__(type)) { - __FACADE_EVENT_TARGET__.addEventListener(type, listener, options); + __FACADE_EVENT_TARGET__.addEventListener( + type, + listener as EventListenerOrEventListenerObject, + options + ); } else { - globalThis.addEventListener(type as any, listener, options); + __facade__originalAddEventListener__(type, listener, options); } }; -globalThis.__facade_removeEventListener__ = function (type, listener, options) { +globalThis.removeEventListener = function (type, listener, options) { if (__facade_isSpecialEvent__(type)) { - __FACADE_EVENT_TARGET__.removeEventListener(type, listener, options); + __FACADE_EVENT_TARGET__.removeEventListener( + type, + listener as EventListenerOrEventListenerObject, + options + ); } else { - globalThis.removeEventListener(type as any, listener, options); + __facade__originalRemoveEventListener__(type, listener, options); } }; -globalThis.__facade_dispatchEvent__ = function (event) { +globalThis.dispatchEvent = function (event) { if (__facade_isSpecialEvent__(event.type)) { - __FACADE_EVENT_TARGET__.dispatchEvent(event); + return __FACADE_EVENT_TARGET__.dispatchEvent(event); } else { - globalThis.dispatchEvent(event as any); + return __facade__originalDispatchEvent__(event); } }; +declare global { + var addMiddleware: typeof __facade_register__; + var addMiddlewareInternal: typeof __facade_registerInternal__; +} +globalThis.addMiddleware = __facade_register__; +globalThis.addMiddlewareInternal = __facade_registerInternal__; + const __facade_waitUntil__ = Symbol("__facade_waitUntil__"); const __facade_response__ = Symbol("__facade_response__"); const __facade_dispatched__ = Symbol("__facade_dispatched__"); @@ -154,7 +168,7 @@ class __Facade_ScheduledEvent__ extends __Facade_ExtendableEvent__ { } } -globalThis.addEventListener("fetch", (event) => { +__facade__originalAddEventListener__("fetch", (event) => { const ctx: ExecutionContext = { waitUntil: event.waitUntil.bind(event), passThroughOnException: event.passThroughOnException.bind(event), @@ -201,7 +215,7 @@ globalThis.addEventListener("fetch", (event) => { ); }); -globalThis.addEventListener("scheduled", (event) => { +__facade__originalAddEventListener__("scheduled", (event) => { const facadeEvent = new __Facade_ScheduledEvent__("scheduled", { scheduledTime: event.scheduledTime, cron: event.cron, diff --git a/packages/wrangler/templates/middleware/middleware-miniflare3-json-error.ts b/packages/wrangler/templates/middleware/middleware-miniflare3-json-error.ts new file mode 100644 index 000000000000..34cb2ac54599 --- /dev/null +++ b/packages/wrangler/templates/middleware/middleware-miniflare3-json-error.ts @@ -0,0 +1,20 @@ +import type { Middleware } from "./common"; + +// See comment in `bundle.ts` for details on why this is needed +const jsonError: Middleware = async (request, env, _ctx, middlewareCtx) => { + try { + return await middlewareCtx.next(request, env); + } catch (e: any) { + const error = { + name: e?.name, + message: e?.message ?? String(e), + stack: e?.stack, + }; + return Response.json(error, { + status: 500, + headers: { "MF-Experimental-Error-Stack": "true" }, + }); + } +}; + +export default jsonError;