From fa5e9cf1b3b828405d822ad6b82cd41742df7291 Mon Sep 17 00:00:00 2001 From: walshy Date: Mon, 12 Oct 2026 16:57:14 -0400 Subject: [PATCH] [wrangler] Upload source maps by default `upload_source_maps` now defaults to on for `wrangler deploy`, `wrangler versions upload` and `wrangler preview` when it isn't set in config or via `--upload-source-maps`, so stack traces point at original source without extra setup. `wrangler build --experimental-cf-build-output` keeps its opt-in behaviour. When defaulted, source maps that can't be resolved (inline `data:` maps, missing `.map` files from custom builds) are skipped with a debug log instead of failing the upload, so existing prebuilt Workers keep deploying. An explicit `upload_source_maps = true` keeps the strict errors, and `false` / `--no-upload-source-maps` opts out. Also resolve Wrangler-bundled source map paths with `path.resolve`, as esbuild reports an absolute output path when `--outdir` is on a different Windows drive, which previously produced an invalid joined path. --- .changeset/default-upload-source-maps.md | 9 + .../workers-utils/src/config/environment.ts | 5 + .../src/__tests__/deploy/build.test.ts | 56 ++++++ .../src/__tests__/deploy/formats.test.ts | 10 ++ .../deployment-bundle/source-maps.test.ts | 47 +++++ .../wrangler/src/__tests__/preview.test.ts | 164 +++++++++--------- .../src/deployment-bundle/build-output.ts | 2 +- .../src/deployment-bundle/deploy-args.ts | 3 +- .../deployment-bundle/maybe-build-worker.ts | 16 +- .../deployment-bundle/merge-config-args.ts | 2 +- .../src/deployment-bundle/source-maps.ts | 47 ++++- 11 files changed, 265 insertions(+), 96 deletions(-) create mode 100644 .changeset/default-upload-source-maps.md diff --git a/.changeset/default-upload-source-maps.md b/.changeset/default-upload-source-maps.md new file mode 100644 index 00000000000..4b656b31883 --- /dev/null +++ b/.changeset/default-upload-source-maps.md @@ -0,0 +1,9 @@ +--- +"wrangler": minor +--- + +Upload source maps by default for `wrangler deploy`, `wrangler versions upload` and `wrangler preview` + +When `upload_source_maps` isn't set in config or via `--upload-source-maps`, Wrangler now generates and uploads source maps so stack traces in logs and `wrangler tail` point at your original source without extra setup. In this default mode, source maps that can't be resolved (for example inline `data:` maps or missing `.map` files from a custom build) are skipped rather than failing the upload. Setting `upload_source_maps = true` keeps the existing strict behaviour, and `upload_source_maps = false` (or `--no-upload-source-maps` for deploy and versions upload) opts out. + +`wrangler build --experimental-cf-build-output` is unchanged and still only includes source maps when `upload_source_maps = true`. diff --git a/packages/workers-utils/src/config/environment.ts b/packages/workers-utils/src/config/environment.ts index a36c56379de..90deb961c34 100644 --- a/packages/workers-utils/src/config/environment.ts +++ b/packages/workers-utils/src/config/environment.ts @@ -830,6 +830,11 @@ interface EnvironmentInheritable { /** * Include source maps when uploading this worker. * + * Defaults to `true` for `wrangler deploy`, `wrangler versions upload` and + * `wrangler preview`, where source maps that can't be resolved are skipped + * rather than failing the upload. Defaults to `false` for + * `wrangler build --experimental-cf-build-output`. + * * For reference, see https://developers.cloudflare.com/workers/wrangler/configuration/#source-maps * * @inheritable diff --git a/packages/wrangler/src/__tests__/deploy/build.test.ts b/packages/wrangler/src/__tests__/deploy/build.test.ts index 592beeed79a..3c08d68b020 100644 --- a/packages/wrangler/src/__tests__/deploy/build.test.ts +++ b/packages/wrangler/src/__tests__/deploy/build.test.ts @@ -1350,6 +1350,62 @@ export default { fetch() { return new Response(mod); } };`; await runWrangler("deploy"); }); + it("should include source map with bundle by default", async ({ + expect, + }) => { + writeWranglerConfig({ main: "index.ts" }); + writeWorkerSource({ format: "ts" }); + mockSubDomainRequest(); + mockUploadWorkerRequest({ + expectedMainModule: "index.js", + expectedModules: { + "index.js.map": expect.stringMatching( + /"sources":\["another.ts","index.ts"\],"sourceRoot":"".*"file":"index.js"/ + ), + }, + }); + + await runWrangler("deploy"); + }); + + it("should skip unresolvable source maps by default rather than failing", async () => { + writeWranglerConfig({ no_bundle: true, main: "index.js" }); + fs.writeFileSync( + "index.js", + `export default { fetch() { return new Response("Hello World"); } }\n` + + "//# sourceMappingURL=missing.js.map" + ); + + mockSubDomainRequest(); + mockUploadWorkerRequest({ + expectedMainModule: "index.js", + expectedModules: { + "index.js.map": null, + }, + }); + + await runWrangler("deploy"); + }); + + it("should fail on unresolvable source maps when upload_source_maps = true", async ({ + expect, + }) => { + writeWranglerConfig({ + no_bundle: true, + main: "index.js", + upload_source_maps: true, + }); + fs.writeFileSync( + "index.js", + `export default { fetch() { return new Response("Hello World"); } }\n` + + "//# sourceMappingURL=missing.js.map" + ); + + await expect(runWrangler("deploy")).rejects.toThrow( + /Invalid source map path in .*index\.js: .*missing\.js\.map does not exist\./ + ); + }); + it("should not include source map with bundle when upload_source_maps = false", async () => { writeWranglerConfig({ main: "index.ts", diff --git a/packages/wrangler/src/__tests__/deploy/formats.test.ts b/packages/wrangler/src/__tests__/deploy/formats.test.ts index 601ff01abd6..3cf64eaf586 100644 --- a/packages/wrangler/src/__tests__/deploy/formats.test.ts +++ b/packages/wrangler/src/__tests__/deploy/formats.test.ts @@ -1166,6 +1166,11 @@ export default{ Content-Type: application/wasm Hello wasm World! + ------formdata-undici-0.test + Content-Disposition: form-data; name="index.js.map"; filename="index.js.map" + Content-Type: application/source-map + + {"version":3,"sources":["index.js"],"sourceRoot":"","sourcesContent":["\\nimport txt from './textfile.txt';\\nimport hello from './hello.wasm';\\nexport default{\\n async fetch(){\\n\\t\\tconst module = await WebAssembly.instantiate(hello);\\n return new Response(txt + module.exports.hello);\\n }\\n}\\n"],"mappings":";AACA,OAAO,SAAS;AAChB,OAAO,WAAW;AAClB,IAAO,gBAAO;AAAA,EACZ,MAAM,QAAO;AACb,UAAM,SAAS,MAAM,YAAY,YAAY,KAAK;AAChD,WAAO,IAAI,SAAS,MAAM,OAAO,QAAQ,KAAK;AAAA,EAChD;AACF;","names":[],"file":"index.js"} ------formdata-undici-0.test-- " `); @@ -1311,6 +1316,11 @@ export default{ Content-Type: application/wasm Hello wasm World! + ------formdata-undici-0.test + Content-Disposition: form-data; name="index.js.map"; filename="index.js.map" + Content-Type: application/source-map + + {"version":3,"sources":["index.js"],"sourceRoot":"","sourcesContent":["\\nimport txt from './textfile.txt';\\nimport hello from './hello.wasm';\\nexport default{\\n async fetch(){\\n\\t\\tconst module = await WebAssembly.instantiate(hello);\\n return new Response(txt + module.exports.hello);\\n }\\n}\\n"],"mappings":";AACA,OAAO,SAAS;AAChB,OAAO,WAAW;AAClB,IAAO,gBAAO;AAAA,EACZ,MAAM,QAAO;AACb,UAAM,SAAS,MAAM,YAAY,YAAY,KAAK;AAChD,WAAO,IAAI,SAAS,MAAM,OAAO,QAAQ,KAAK;AAAA,EAChD;AACF;","names":[],"file":"index.js"} ------formdata-undici-0.test-- " `); diff --git a/packages/wrangler/src/__tests__/deployment-bundle/source-maps.test.ts b/packages/wrangler/src/__tests__/deployment-bundle/source-maps.test.ts index 45790c45f15..562541cc250 100644 --- a/packages/wrangler/src/__tests__/deployment-bundle/source-maps.test.ts +++ b/packages/wrangler/src/__tests__/deployment-bundle/source-maps.test.ts @@ -51,6 +51,29 @@ describe("loadSourceMaps", () => { expect(result[0].name).toBe("src/index.ts.map"); }); + it("loads bundled source maps when the output path is absolute", ({ + expect, + }) => { + const entryDir = createTempDir(); + const outDir = createTempDir(); + writeFileSync(path.join(outDir, "index.js.map"), validSourceMap); + + const main = makeModule( + "index.js", + path.join(entryDir, "index.js"), + "const x = 1;\n" + ); + const bundle: SourceMapBundle = { + sourceMapPath: path.join(outDir, "index.js.map"), + sourceMapMetadata: { tmpDir: outDir, entryDirectory: entryDir }, + }; + + const result = loadSourceMaps(main, [], bundle); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe("index.js.map"); + }); + it("throws when bundled source map file is missing", ({ expect }) => { const dir = createTempDir(); const main = makeModule( @@ -85,6 +108,30 @@ describe("loadSourceMaps", () => { expect(result[0].name).toBe("index.js.map"); }); + it("skips unresolvable source maps in scan mode when not strict", ({ + expect, + }) => { + const dir = createTempDir(); + writeFileSync(path.join(dir, "other.js.map"), validSourceMap); + + const main = makeModule( + "index.js", + path.join(dir, "index.js"), + `console.log("hello");\n//# sourceMappingURL=missing.js.map` + ); + const other = makeModule( + "other.js", + path.join(dir, "other.js"), + `console.log("other");\n//# sourceMappingURL=other.js.map` + ); + + expect(() => loadSourceMaps(main, [other], {})).toThrow(); + + const result = loadSourceMaps(main, [other], {}, { strict: false }); + + expect(result.map((sourceMap) => sourceMap.name)).toEqual(["other.js.map"]); + }); + it("handles multiple modules with source maps in scan mode", ({ expect }) => { const dir = createTempDir(); writeFileSync(path.join(dir, "a.js.map"), validSourceMap); diff --git a/packages/wrangler/src/__tests__/preview.test.ts b/packages/wrangler/src/__tests__/preview.test.ts index eeec2eb80b4..747ce074d7e 100644 --- a/packages/wrangler/src/__tests__/preview.test.ts +++ b/packages/wrangler/src/__tests__/preview.test.ts @@ -6140,93 +6140,97 @@ describe("wrangler preview", () => { ); }); - test("should include source maps in deployment modules when upload_source_maps is enabled", async ({ - expect, - }) => { - writeFileSync( - "wrangler.json", - JSON.stringify({ - name: "test-worker", - main: "src/index.ts", - compatibility_date: "2025-01-01", - upload_source_maps: true, - previews: {}, - }) - ); + test.for([ + { label: "enabled", uploadSourceMaps: true }, + { label: "unset", uploadSourceMaps: undefined }, + ])( + "should include source maps in deployment modules when upload_source_maps is $label", + async ({ uploadSourceMaps }, { expect }) => { + writeFileSync( + "wrangler.json", + JSON.stringify({ + name: "test-worker", + main: "src/index.ts", + compatibility_date: "2025-01-01", + upload_source_maps: uploadSourceMaps, + previews: {}, + }) + ); - let deploymentRequestBody: - | (Record & { - modules?: PreviewDeploymentModulePart[]; - }) - | undefined; + let deploymentRequestBody: + | (Record & { + modules?: PreviewDeploymentModulePart[]; + }) + | undefined; - msw.use( - http.get( - `*/accounts/:accountId/workers/workers/:workerId/previews/:previewId`, - () => - HttpResponse.json( - { - success: false, - result: null, - errors: [{ code: 10025, message: "Preview not found" }], - }, - { status: 404 } - ) - ), - http.post( - `*/accounts/:accountId/workers/workers/:workerId/previews`, - () => - HttpResponse.json( - { - success: true, - result: { - id: "preview-id-sourcemaps", - name: "test-preview", - slug: "test-preview", - urls: ["https://test-preview.test-worker.cloudflare.app"], - worker_name: "test-worker", - created_on: new Date().toISOString(), + msw.use( + http.get( + `*/accounts/:accountId/workers/workers/:workerId/previews/:previewId`, + () => + HttpResponse.json( + { + success: false, + result: null, + errors: [{ code: 10025, message: "Preview not found" }], }, - }, - { status: 201 } - ) - ), - http.post( - `*/accounts/:accountId/workers/workers/:workerId/previews/:previewId/deployments`, - async ({ request }) => { - deploymentRequestBody = (await readPreviewDeploymentRequest( - request - )) as typeof deploymentRequestBody; - return HttpResponse.json( - { - success: true, - result: { - id: "deployment-id-sourcemaps", - preview_id: "preview-id-sourcemaps", - preview_name: "test-preview", - urls: ["https://sourcemaps123.test-worker.cloudflare.app"], - compatibility_date: "2025-01-01", - env: {}, - created_on: new Date().toISOString(), + { status: 404 } + ) + ), + http.post( + `*/accounts/:accountId/workers/workers/:workerId/previews`, + () => + HttpResponse.json( + { + success: true, + result: { + id: "preview-id-sourcemaps", + name: "test-preview", + slug: "test-preview", + urls: ["https://test-preview.test-worker.cloudflare.app"], + worker_name: "test-worker", + created_on: new Date().toISOString(), + }, }, - }, - { status: 201 } - ); - } - ) - ); + { status: 201 } + ) + ), + http.post( + `*/accounts/:accountId/workers/workers/:workerId/previews/:previewId/deployments`, + async ({ request }) => { + deploymentRequestBody = (await readPreviewDeploymentRequest( + request + )) as typeof deploymentRequestBody; + return HttpResponse.json( + { + success: true, + result: { + id: "deployment-id-sourcemaps", + preview_id: "preview-id-sourcemaps", + preview_name: "test-preview", + urls: ["https://sourcemaps123.test-worker.cloudflare.app"], + compatibility_date: "2025-01-01", + env: {}, + created_on: new Date().toISOString(), + }, + }, + { status: 201 } + ); + } + ) + ); - await runWrangler("preview --name test-preview"); + await runWrangler("preview --name test-preview"); - const sourceMap = deploymentRequestBody?.modules?.find((module) => - module.name.endsWith(".map") - ); + const sourceMap = deploymentRequestBody?.modules?.find((module) => + module.name.endsWith(".map") + ); - expect(sourceMap?.content_type).toBe("application/source-map"); - expect(JSON.parse(decodeModuleContent(sourceMap))).toEqual( - expect.objectContaining({ version: 3 }) - ); - }); + expect(sourceMap?.content_type).toBe("application/source-map"); + expect(JSON.parse(decodeModuleContent(sourceMap))).toEqual( + expect.objectContaining({ version: 3 }) + ); + } + ); test("should use previews.define for worker bundling", async ({ expect, diff --git a/packages/wrangler/src/deployment-bundle/build-output.ts b/packages/wrangler/src/deployment-bundle/build-output.ts index 68c9db04a80..6cbf7fdb722 100644 --- a/packages/wrangler/src/deployment-bundle/build-output.ts +++ b/packages/wrangler/src/deployment-bundle/build-output.ts @@ -80,7 +80,7 @@ async function writeBundle({ } // Source maps. `buildResult.sourceMaps` is already gated by - // `uploadSourceMaps` inside `buildWorker` (undefined when off) and + // `uploadSourceMaps` inside `buildWorker` (undefined when explicitly off) and // contains the main module's map plus any module maps, each named // `.map` (e.g. `index.js.map`). for (const sourceMap of buildResult.sourceMaps ?? []) { diff --git a/packages/wrangler/src/deployment-bundle/deploy-args.ts b/packages/wrangler/src/deployment-bundle/deploy-args.ts index f610fe97f54..0ef23544b8c 100644 --- a/packages/wrangler/src/deployment-bundle/deploy-args.ts +++ b/packages/wrangler/src/deployment-bundle/deploy-args.ts @@ -143,7 +143,8 @@ export const sharedDeployVersionsArgs = { type: "boolean", }, "upload-source-maps": { - describe: "Include source maps when uploading this Worker", + describe: + "Include source maps when uploading this Worker (defaults to true)", type: "boolean", }, "node-compat": { diff --git a/packages/wrangler/src/deployment-bundle/maybe-build-worker.ts b/packages/wrangler/src/deployment-bundle/maybe-build-worker.ts index 49bc8c6c11c..31f05e3b9aa 100644 --- a/packages/wrangler/src/deployment-bundle/maybe-build-worker.ts +++ b/packages/wrangler/src/deployment-bundle/maybe-build-worker.ts @@ -36,7 +36,10 @@ export type BuildProps = { compatibilityDate: string | undefined; /** Merged: --compatibility-flags arg ?? config.compatibility_flags. */ compatibilityFlags: string[]; - /** Merged: --upload-source-maps arg ?? config.upload_source_maps. */ + /** + * Merged: --upload-source-maps arg ?? config.upload_source_maps. + * `undefined` means source maps are uploaded on a best-effort basis. + */ uploadSourceMaps: boolean | undefined; /** Merged: --jsx-factory arg || config.jsx_factory. */ jsxFactory: string; @@ -158,7 +161,7 @@ export async function buildWorker( tsconfig: props.tsconfig, minify, keepNames: config.keep_names ?? true, - sourcemap: uploadSourceMaps, + sourcemap: uploadSourceMaps ?? true, nodejsCompatMode, compatibilityDate, compatibilityFlags, @@ -210,9 +213,12 @@ export async function buildWorker( content, type: bundleType, }; - const sourceMaps = uploadSourceMaps - ? loadSourceMaps(main, modules, bundle) - : undefined; + const sourceMaps = + uploadSourceMaps === false + ? undefined + : loadSourceMaps(main, modules, bundle, { + strict: uploadSourceMaps === true, + }); return { modules, diff --git a/packages/wrangler/src/deployment-bundle/merge-config-args.ts b/packages/wrangler/src/deployment-bundle/merge-config-args.ts index a11cdbf0e0e..d1396ef35fb 100644 --- a/packages/wrangler/src/deployment-bundle/merge-config-args.ts +++ b/packages/wrangler/src/deployment-bundle/merge-config-args.ts @@ -253,7 +253,7 @@ export async function mergeBuildOutputProps(config: Config): Promise<{ name: config.name, compatibilityDate: config.compatibility_date, compatibilityFlags: config.compatibility_flags, - uploadSourceMaps: config.upload_source_maps, + uploadSourceMaps: config.upload_source_maps ?? false, jsxFactory: config.jsx_factory, jsxFragment: config.jsx_fragment, tsconfig: config.tsconfig, diff --git a/packages/wrangler/src/deployment-bundle/source-maps.ts b/packages/wrangler/src/deployment-bundle/source-maps.ts index e44eae07d5b..8973654c0ff 100644 --- a/packages/wrangler/src/deployment-bundle/source-maps.ts +++ b/packages/wrangler/src/deployment-bundle/source-maps.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import { logger } from "../logger"; import type { CfModule, CfWorkerSourceMap } from "@cloudflare/workers-utils"; interface SourceMapMetadata { @@ -15,11 +16,20 @@ export interface SourceMapBundle { /** * Loads source maps that appear in the given build output. + * + * @param main The main module of the Worker. + * @param modules Any additional modules of the Worker. + * @param bundle Source map location metadata from Wrangler's bundler, if it ran. + * @param options.strict When `false`, modules whose source map comment can't be + * resolved are skipped rather than failing. Used when source maps are uploaded + * by default rather than explicitly requested. + * @returns The source maps to upload alongside the Worker. */ export function loadSourceMaps( main: CfModule, modules: CfModule[], - bundle: SourceMapBundle + bundle: SourceMapBundle, + { strict = true }: { strict?: boolean } = {} ): CfWorkerSourceMap[] { const { sourceMapPath, sourceMapMetadata } = bundle; if (sourceMapPath && sourceMapMetadata) { @@ -29,7 +39,7 @@ export function loadSourceMaps( } else { // Don't know where source maps are located, so need to find // them by scanning the contents of the user-specified modules. - return scanSourceMaps([main, ...modules]); + return scanSourceMaps([main, ...modules], strict); } } @@ -45,8 +55,13 @@ function loadSourceMap( if (filePath === undefined) { return []; } + + // esbuild reports output paths relative to the entry directory, except when + // no relative path exists (e.g. an `--outdir` on another Windows drive), + // in which case the path is absolute. + const resolvedSourceMapPath = path.resolve(entryDirectory, sourceMapPath); const map = JSON.parse( - fs.readFileSync(path.join(entryDirectory, sourceMapPath), "utf8") + fs.readFileSync(resolvedSourceMapPath, "utf8") ) as RawSourceMap; // Overwrite the file property of the source map to match the name of the // main module in the multipart upload. @@ -54,9 +69,7 @@ function loadSourceMap( if (map.sourceRoot) { // Remove the temporary directory prefix generated by Wrangler that appears // in the source root path. - const sourceRootPath = path.dirname( - path.join(entryDirectory, sourceMapPath) - ); + const sourceRootPath = path.dirname(resolvedSourceMapPath); map.sourceRoot = path.relative(sourceRootPath, map.sourceRoot); } map.sources = map.sources.map((source) => { @@ -75,10 +88,15 @@ function loadSourceMap( * Find source maps by scanning module contents for special `//# * sourceMappingURL=` comments pointing to source map files. */ -function scanSourceMaps(modules: CfModule[]): CfWorkerSourceMap[] { +function scanSourceMaps( + modules: CfModule[], + strict: boolean +): CfWorkerSourceMap[] { const maps: CfWorkerSourceMap[] = []; for (const module of modules) { - const maybeSourcemap = sourceMapForModule(module); + const maybeSourcemap = strict + ? sourceMapForModule(module) + : trySourceMapForModule(module); if (maybeSourcemap) { maps.push(maybeSourcemap); } @@ -86,6 +104,19 @@ function scanSourceMaps(modules: CfModule[]): CfWorkerSourceMap[] { return maps; } +function trySourceMapForModule( + module: CfModule +): CfWorkerSourceMap | undefined { + try { + return sourceMapForModule(module); + } catch (error) { + logger.debug( + `Skipping source map for ${module.name}: ${error instanceof Error ? error.message : String(error)}` + ); + return undefined; + } +} + /** * Attaches a sourcemap, if found, to a JavaScript module. */