diff --git a/.changeset/olive-chefs-sleep.md b/.changeset/olive-chefs-sleep.md new file mode 100644 index 000000000000..341c824dcf62 --- /dev/null +++ b/.changeset/olive-chefs-sleep.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: apply source mapping to deployment validation errors + +Previously if a Worker failed validation during `wrangler deploy`, the displayed error would reference locations in built JavaScript files. This made it more difficult to debug validation errors. This change ensures these errors are now source mapped, referencing locations in source files instead. diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 6d3c68d30d7e..7715e74cf2dd 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -7,6 +7,7 @@ import * as TOML from "@iarna/toml"; import commandExists from "command-exists"; import * as esbuild from "esbuild"; import { MockedRequest, rest } from "msw"; +import dedent from "ts-dedent"; import { FormData } from "undici"; import { printBundleSize, @@ -1835,6 +1836,119 @@ addEventListener('fetch', event => {});` } `); }); + + describe("should source map validation errors", () => { + function mockDeployWithValidationError(message: string) { + const handler = rest.put( + "*/accounts/:accountId/workers/scripts/:scriptName", + async (req, res, ctx) => { + const body = createFetchResult(null, false, [ + { code: 10021, message }, + ]); + return res(ctx.json(body)); + } + ); + msw.use(handler); + } + + it("with TypeScript source file", async () => { + writeWranglerToml(); + fs.writeFileSync( + `index.ts`, + dedent`interface Env { + THING: string; + } + x; + export default { + fetch() { + return new Response("body"); + } + }` + ); + mockDeployWithValidationError( + "Uncaught ReferenceError: x is not defined\n at index.js:2:1\n" + ); + mockSubDomainRequest(); + + await expect(runWrangler("deploy ./index.ts")).rejects.toMatchObject({ + notes: [{ text: expect.stringContaining("index.ts:4:1") }, {}], + }); + }); + + it("with additional modules", async () => { + writeWranglerToml({ + no_bundle: true, + rules: [{ type: "ESModule", globs: ["**/*.js"] }], + }); + + fs.writeFileSync( + "dep.ts", + dedent`interface Env { + } + y; + export default "message";` + ); + await esbuild.build({ + bundle: true, + format: "esm", + entryPoints: [path.resolve("dep.ts")], + outdir: process.cwd(), + sourcemap: true, + }); + + fs.writeFileSync( + "index.js", + dedent`import dep from "./dep.js"; + export default { + fetch() { + return new Response(dep); + } + }` + ); + + mockDeployWithValidationError( + "Uncaught ReferenceError: y is not defined\n at dep.js:2:1\n" + ); + mockSubDomainRequest(); + + await expect(runWrangler("deploy ./index.js")).rejects.toMatchObject({ + notes: [{ text: expect.stringContaining("dep.ts:3:1") }, {}], + }); + }); + + it("with inline source map", async () => { + writeWranglerToml({ + no_bundle: true, + }); + + fs.writeFileSync( + "index.ts", + dedent`interface Env {} + z; + export default { + fetch() { + return new Response("body"); + } + }` + ); + await esbuild.build({ + bundle: true, + format: "esm", + entryPoints: [path.resolve("index.ts")], + outdir: process.cwd(), + sourcemap: "inline", + }); + + mockDeployWithValidationError( + "Uncaught ReferenceError: z is not defined\n at index.js:2:1\n" + ); + mockSubDomainRequest(); + + await expect(runWrangler("deploy ./index.js")).rejects.toMatchObject({ + notes: [{ text: expect.stringContaining("index.ts:2:1") }, {}], + }); + }); + }); }); describe("asset upload", () => { diff --git a/packages/wrangler/src/deploy/deploy.ts b/packages/wrangler/src/deploy/deploy.ts index 3c68c7d5c86b..df0aa1fb0e5c 100644 --- a/packages/wrangler/src/deploy/deploy.ts +++ b/packages/wrangler/src/deploy/deploy.ts @@ -30,6 +30,10 @@ import { getWranglerTmpDir } from "../paths"; import { getQueue, putConsumer } from "../queues/client"; import { getWorkersDevSubdomain } from "../routes"; import { syncAssets } from "../sites"; +import { + getSourceMappedString, + maybeRetrieveFileSourceMap, +} from "../sourcemap"; import { getZoneForRoute } from "../zones"; import type { FetchError } from "../cfetch"; import type { Config } from "../config"; @@ -44,6 +48,7 @@ import type { Entry } from "../deployment-bundle/entry"; import type { CfWorkerInit, CfPlacement } from "../deployment-bundle/worker"; import type { PutConsumerBody } from "../queues/client"; import type { AssetPaths } from "../sites"; +import type { RetrieveSourceMapFunction } from "../sourcemap"; type Props = { config: Config; @@ -593,10 +598,11 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m const placement: CfPlacement | undefined = config.placement?.mode === "smart" ? { mode: "smart" } : undefined; + const entryPointName = path.basename(resolvedEntryPointPath); const worker: CfWorkerInit = { name: scriptName, main: { - name: path.basename(resolvedEntryPointPath), + name: entryPointName, filePath: resolvedEntryPointPath, content: content, type: bundleType, @@ -689,6 +695,34 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m } } catch (err) { helpIfErrorIsSizeOrScriptStartup(err, dependencies); + + // Apply source mapping to validation startup errors if possible + if ( + err instanceof ParseError && + "code" in err && + err.code === 10021 /* validation error */ && + err.notes.length > 0 + ) { + const maybeNameToFilePath = (moduleName: string) => { + // If this is a service worker, always return the entrypoint path. + // Service workers can't have additional JavaScript modules. + if (bundleType === "commonjs") return resolvedEntryPointPath; + // Similarly, if the name matches the entrypoint, return its path + if (moduleName === entryPointName) return resolvedEntryPointPath; + // Otherwise, return the file path of the matching module (if any) + for (const module of modules) { + if (moduleName === module.name) return module.filePath; + } + }; + const retrieveSourceMap: RetrieveSourceMapFunction = (moduleName) => + maybeRetrieveFileSourceMap(maybeNameToFilePath(moduleName)); + + err.notes[0].text = getSourceMappedString( + err.notes[0].text, + retrieveSourceMap + ); + } + throw err; } } diff --git a/packages/wrangler/src/sourcemap.ts b/packages/wrangler/src/sourcemap.ts index ebce3abe67b6..4ba17ce0384a 100644 --- a/packages/wrangler/src/sourcemap.ts +++ b/packages/wrangler/src/sourcemap.ts @@ -1,10 +1,69 @@ import assert from "node:assert"; +import fs from "node:fs"; +import url from "node:url"; +import type { Options } from "@cspotcode/source-map-support"; import type Protocol from "devtools-protocol"; +function maybeGetFile(filePath: string | URL) { + try { + return fs.readFileSync(filePath, "utf8"); + } catch (e: unknown) { + const notFound = + typeof e === "object" && e !== null && "code" in e && e.code === "ENOENT"; + if (!notFound) throw e; + } +} + +export type RetrieveSourceMapFunction = NonNullable< + Options["retrieveSourceMap"] +>; +export function maybeRetrieveFileSourceMap( + filePath?: string +): ReturnType { + if (filePath === undefined) return null; + const contents = maybeGetFile(filePath); + if (contents === undefined) return null; + + // Find the last source mapping URL if any + const mapRegexp = /# sourceMappingURL=(.+)/g; + const matches = [...contents.matchAll(mapRegexp)]; + // If we couldn't find a source mapping URL, there's nothing we can do + if (matches.length === 0) return null; + const mapMatch = matches[matches.length - 1]; + + // Get the source map + const fileUrl = url.pathToFileURL(filePath); + const mapUrl = new URL(mapMatch[1], fileUrl); + if ( + mapUrl.protocol === "data:" && + mapUrl.pathname.startsWith("application/json;base64,") + ) { + // sourceMappingURL=data:application/json;base64,ew... + const base64 = mapUrl.href.substring(mapUrl.href.indexOf(",") + 1); + const map = Buffer.from(base64, "base64").toString(); + return { map, url: fileUrl.href }; + } else { + const map = maybeGetFile(mapUrl); + if (map === undefined) return null; + return { map, url: mapUrl.href }; + } +} + +// `sourceMappingPrepareStackTrace` is initialised on the first call to +// `getSourceMappingPrepareStackTrace()`. Subsequent calls to +// `getSourceMappingPrepareStackTrace()` will not update it. We'd like to be +// able to customise source map retrieval on each call though. Therefore, we +// make `retrieveSourceMapOverride` a module level variable, so +// `sourceMappingPrepareStackTrace` always has access to the latest override. let sourceMappingPrepareStackTrace: typeof Error.prepareStackTrace; -function getSourceMappingPrepareStackTrace(): NonNullable< - typeof Error.prepareStackTrace -> { +let retrieveSourceMapOverride: RetrieveSourceMapFunction | undefined; + +function getSourceMappingPrepareStackTrace( + retrieveSourceMap?: RetrieveSourceMapFunction +): NonNullable { + // Source mapping is synchronous, so setting a module level variable is fine + retrieveSourceMapOverride = retrieveSourceMap; + // If we already have a source mapper, return it if (sourceMappingPrepareStackTrace !== undefined) { return sourceMappingPrepareStackTrace; } @@ -31,6 +90,10 @@ function getSourceMappingPrepareStackTrace(): NonNullable< redirectConflictingLibrary: false, // Make sure we're using fresh copies of files each time we source map emptyCacheBetweenOperations: true, + // Allow retriever to be overridden at prepare stack trace time + retrieveSourceMap(path) { + return retrieveSourceMapOverride?.(path) ?? null; + }, }); sourceMappingPrepareStackTrace = Error.prepareStackTrace; assert(sourceMappingPrepareStackTrace !== undefined); @@ -69,7 +132,10 @@ function callFrameToCallSite(frame: Protocol.Runtime.CallFrame): CallSite { } const placeholderError = new Error(); -export function getSourceMappedString(value: string): string { +export function getSourceMappedString( + value: string, + retrieveSourceMap?: RetrieveSourceMapFunction +): string { // We could use `.replace()` here with a function replacer, but // `getSourceMappingPrepareStackTrace()` clears its source map caches between // operations. It's likely call sites in this `value` will share source maps, @@ -80,7 +146,8 @@ export function getSourceMappedString(value: string): string { // of the call site would've been replaced with the same thing. const callSiteLines = Array.from(value.matchAll(CALL_SITE_REGEXP)); const callSites = callSiteLines.map(lineMatchToCallSite); - const sourceMappedStackTrace: string = getSourceMappingPrepareStackTrace()( + const prepareStack = getSourceMappingPrepareStackTrace(retrieveSourceMap); + const sourceMappedStackTrace: string = prepareStack( placeholderError, callSites );