diff --git a/dev-packages/e2e-tests/test-applications/nuxt-4-sourcemaps/assert-build.ts b/dev-packages/e2e-tests/test-applications/nuxt-4-sourcemaps/assert-build.ts index 14d64149cefc..49b9839d98ba 100644 --- a/dev-packages/e2e-tests/test-applications/nuxt-4-sourcemaps/assert-build.ts +++ b/dev-packages/e2e-tests/test-applications/nuxt-4-sourcemaps/assert-build.ts @@ -2,6 +2,9 @@ import * as assert from 'assert/strict'; import * as fs from 'fs'; import * as path from 'path'; import { + findDebugIdsWithConflictingSourcemaps, + findFilesWithForeignSourcemaps, + findFilesWithMultipleDebugIds, findInjectedDebugIds, findSourceMapFiles, findSourceMappingUrlComments, @@ -9,6 +12,7 @@ import { getAssembleRequests, getChunkUploadPosts, getDebugIdPairs, + getSourcemapSources, getSourcemaps, loadMockServerResults, } from '@sentry-internal/test-utils'; @@ -173,6 +177,36 @@ for (const outputDir of isStaticBuild ? [CLIENT_OUTPUT] : [CLIENT_OUTPUT, SERVER assert.deepEqual(unuploaded, [], `Expected every debug ID in ${outputDir} to have an uploaded sourcemap`); console.log(` ${outputDir}: ${injectedDebugIds.length} injected debug ID(s), all uploaded`); + + // Nitro bundles the SSR build again, so its debug IDs can end up next to Nitro's own. + // Reference issue: https://github.com/getsentry/sentry-javascript/issues/18519 + const filesWithMultipleDebugIds = findFilesWithMultipleDebugIds({ outputDir }); + assert.deepEqual(filesWithMultipleDebugIds, [], `Expected at most one debug ID per file in ${outputDir}`); + + // A server file can borrow the debug ID of its SSR build chunk. Then the upload check above passes on + // Vite's map even if Nitro uploaded nothing. + const filesWithForeignSourcemaps = findFilesWithForeignSourcemaps({ outputDir }, sourcemaps); + assert.deepEqual( + filesWithForeignSourcemaps, + [], + `Expected the source map uploaded for each file in ${outputDir} to be generated for that file`, + ); +} + +// If both builds upload a map under a borrowed debug ID, Sentry can resolve with the wrong one. +const conflictingDebugIds = findDebugIdsWithConflictingSourcemaps(sourcemaps); +assert.deepEqual(conflictingDebugIds, [], 'Expected every uploaded debug ID to resolve to exactly one source map'); + +if (!isStaticBuild) { + // Nitro bundles Vite's SSR output, so server frames reach `.vue` files only through the maps of that build. + const serverDebugIds = new Set(findInjectedDebugIds({ outputDir: SERVER_OUTPUT })); + const serverSources = sourcemaps + .filter(map => map.debugId && serverDebugIds.has(map.debugId)) + .flatMap(map => getSourcemapSources(map.sourcemap)); + assert.ok( + serverSources.some(source => source.endsWith('app/pages/index.vue')), + `Expected the server source maps to resolve to app/pages/index.vue, got: ${serverSources.join(', ')}`, + ); } console.log(''); diff --git a/dev-packages/test-utils/src/build-output.ts b/dev-packages/test-utils/src/build-output.ts index a3c2445e9f8a..f42e5dd1bdb0 100644 --- a/dev-packages/test-utils/src/build-output.ts +++ b/dev-packages/test-utils/src/build-output.ts @@ -1,5 +1,6 @@ import * as fs from 'fs'; import * as path from 'path'; +import type { SourcemapEntry } from './sourcemap-upload-utils'; export interface OutputScanOptions { /** Directory holding the emitted bundles, e.g. `/.output/public`. */ @@ -18,6 +19,12 @@ const SOURCE_MAPPING_URL_PATTERN = /[#@]\s*sourceMappingURL\s*=\s*(?!data:)(\S+) /** The `sentry-dbid-` identifier the bundler plugin injects alongside `_sentryDebugIds`. */ const INJECTED_DEBUG_ID_PATTERN = /sentry-dbid-([\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12})/gi; +/** Distinct lowercased debug IDs in `file`, in order of appearance. */ +function readDebugIds(file: string): string[] { + const matches = fs.readFileSync(file, 'utf8').matchAll(INJECTED_DEBUG_ID_PATTERN); + return [...new Set([...matches].map(match => (match[1] as string).toLowerCase()))]; +} + function* walkFiles(outputDir: string, extensions?: string[]): Generator { if (!fs.existsSync(outputDir)) { throw new Error(`[build-output] Output directory does not exist: ${outputDir}`); @@ -147,12 +154,59 @@ export function findInjectedDebugIds({ outputDir, extensions = JS_EXTENSIONS }: const debugIds = new Set(); for (const file of walkFiles(outputDir, extensions)) { - const contents = fs.readFileSync(file, 'utf8'); + readDebugIds(file).forEach(debugId => debugIds.add(debugId)); + } + + return [...debugIds]; +} + +/** + * Returns ` → ` for each file with more than one debug ID. + * The uploader reads the first ID, but the SDK can report any of them. + */ +export function findFilesWithMultipleDebugIds({ + outputDir, + buildDir = process.cwd(), + extensions = JS_EXTENSIONS, +}: OutputScanOptions): string[] { + const offenders: string[] = []; + + for (const file of walkFiles(outputDir, extensions)) { + const debugIds = readDebugIds(file); - for (const match of contents.matchAll(INJECTED_DEBUG_ID_PATTERN)) { - debugIds.add((match[1] as string).toLowerCase()); + if (debugIds.length > 1) { + offenders.push(`${path.relative(buildDir, file)} → ${debugIds.join(', ')}`); } } - return [...debugIds]; + return offenders; +} + +/** + * Returns ` → ` for each file where no map uploaded under its first debug ID names it in `file`. + * Skips files without a debug ID or without an uploaded map that has `file`. + */ +export function findFilesWithForeignSourcemaps( + { outputDir, buildDir = process.cwd(), extensions = JS_EXTENSIONS }: OutputScanOptions, + uploadedSourcemaps: SourcemapEntry[], +): string[] { + const offenders: string[] = []; + + for (const file of walkFiles(outputDir, extensions)) { + const [debugId] = readDebugIds(file); + if (!debugId) { + continue; + } + + const mapFiles = uploadedSourcemaps + .filter(entry => entry.debugId === debugId) + .map(entry => entry.sourcemap.file) + .filter(mapFile => mapFile !== undefined); + + if (mapFiles.length > 0 && !mapFiles.includes(path.basename(file))) { + offenders.push(`${path.relative(buildDir, file)} → ${mapFiles.join(', ')}`); + } + } + + return offenders; } diff --git a/dev-packages/test-utils/src/index.ts b/dev-packages/test-utils/src/index.ts index e1b46f8a5818..3da52584017a 100644 --- a/dev-packages/test-utils/src/index.ts +++ b/dev-packages/test-utils/src/index.ts @@ -23,6 +23,8 @@ export { findSourceMapFiles, findSourceMappingUrlComments, findInjectedDebugIds, + findFilesWithMultipleDebugIds, + findFilesWithForeignSourcemaps, bundleReferencesModule, } from './build-output'; export type { OutputScanOptions } from './build-output'; diff --git a/dev-packages/test-utils/src/sourcemap-upload-utils.ts b/dev-packages/test-utils/src/sourcemap-upload-utils.ts index 88ee298824ac..559169f05bfc 100644 --- a/dev-packages/test-utils/src/sourcemap-upload-utils.ts +++ b/dev-packages/test-utils/src/sourcemap-upload-utils.ts @@ -58,6 +58,7 @@ export interface ArtifactBundleData { export interface ParsedSourcemap { [key: string]: unknown; version?: number; + file?: string; sources?: string[]; /** Absent when the generator drops all sources (Rollup's `sourcemapExcludeSources`), null per dropped entry. */ sourcesContent?: (string | null)[]; @@ -68,6 +69,8 @@ export interface ParsedSourcemap { export interface SourcemapEntry { url: string; bundleDir: string; + /** Lowercased `debug-id` manifest header. */ + debugId?: string; sourcemap: ParsedSourcemap; } @@ -168,6 +171,28 @@ export function getDebugIdPairs(bundles: ArtifactBundleData[]): DebugIdPair[] { return pairs; } +/** + * Returns ` → ` for each debug ID uploaded with maps that differ in `sources` or `mappings`. + * Byte-identical chunks share a debug ID, so the `file` field is not compared. + */ +export function findDebugIdsWithConflictingSourcemaps(sourcemaps: SourcemapEntry[]): string[] { + const mapsByDebugId = new Map>(); + + for (const { url, debugId, sourcemap } of sourcemaps) { + if (!debugId) continue; + + const mappingKey = JSON.stringify([getSourcemapSources(sourcemap), sourcemap.mappings]); + + const maps = mapsByDebugId.get(debugId) ?? new Map(); + maps.set(mappingKey, sourcemap.file ?? url); + mapsByDebugId.set(debugId, maps); + } + + return [...mapsByDebugId] + .filter(([, maps]) => maps.size > 1) + .map(([debugId, maps]) => `${debugId} → ${[...maps.values()].join(', ')}`); +} + /** * Read and parse all sourcemap files from artifact bundles. */ @@ -195,7 +220,8 @@ export function getSourcemaps(bundles: ArtifactBundleData[]): SourcemapEntry[] { continue; } - sourcemaps.push({ url: entry.url, bundleDir, sourcemap }); + const debugId = getManifestHeader(entry.headers, 'debug-id')?.toLowerCase(); + sourcemaps.push({ url: entry.url, bundleDir, debugId, sourcemap }); } } diff --git a/packages/nuxt/src/vite/sourceMaps.ts b/packages/nuxt/src/vite/sourceMaps.ts index de7d7bb00bdc..d03c725e6c8c 100644 --- a/packages/nuxt/src/vite/sourceMaps.ts +++ b/packages/nuxt/src/vite/sourceMaps.ts @@ -22,7 +22,7 @@ export type SourceMapSetting = boolean | 'hidden' | 'inline'; export function setupSourceMaps( moduleOptions: SentryNuxtModuleOptions, nuxt: Nuxt, - addVitePlugin: (plugin: Plugin[], options?: { dev?: boolean; build?: boolean }) => void, + addVitePlugin: (plugin: Plugin[], options?: { dev?: boolean; build?: boolean; server?: boolean }) => void, ): void { // Warn here rather than in `getPluginOptions`, which runs once per bundler (Vite and Nitro's // Rollup) and would emit the same warning twice. @@ -73,13 +73,16 @@ export function setupSourceMaps( }); if (sourceMapsEnabled && !nuxt.options.dev && !nuxt.options?._prepare) { + // Also on the SSR build: Nitro's source maps reach the original files only through that build's maps. + addVitePlugin([validateSourceMapsOptionsPlugin({ nuxt, moduleOptions, sourceMapsEnabled })], { + dev: false, + build: true, + }); + addVitePlugin( - [ - validateSourceMapsOptionsPlugin({ nuxt, moduleOptions, sourceMapsEnabled }), - // Vite plugin is added on the client and server side (plugin runs for both builds) - ...sentryVitePlugin(withoutSourceMapDeletion(getPluginOptions(moduleOptions, shouldDeleteFilesFallback))), - ], - { dev: false, build: true }, // Only add source map plugin during build + sentryVitePlugin(withoutSourceMapDeletion(getPluginOptions(moduleOptions, shouldDeleteFilesFallback))), + // Nitro bundles the SSR output again, so debug IDs from the SSR build would point to the wrong maps. + { dev: false, build: true, server: false }, ); } @@ -162,7 +165,7 @@ export function getPluginOptions( sourcemaps: { disable: moduleOptions.sourcemaps?.disable, // The server/client files are in different places depending on the nitro preset (e.g. '.output/server' or '.netlify/functions-internal/server') - // We cannot determine automatically how the build folder looks like (depends on the preset), so we have to accept that source maps are uploaded multiple times (with the vitePlugin for Nuxt and the rollupPlugin for Nitro). + // We cannot determine automatically how the build folder looks like (depends on the preset), so each plugin uploads its own build output (the vitePlugin for the Nuxt client and the rollupPlugin for Nitro). // If we could know where the server/client assets are located, we could do something like this (based on the Nitro preset): isNitro ? ['./.output/server/**/*'] : ['./.output/public/**/*'], assets: sourcemapsOptions.assets ?? undefined, ignore: sourcemapsOptions.ignore ?? undefined, diff --git a/packages/nuxt/test/vite/sourceMaps-nuxtHooks.test.ts b/packages/nuxt/test/vite/sourceMaps-nuxtHooks.test.ts index 2ef55c0959ec..18f67df7ab40 100644 --- a/packages/nuxt/test/vite/sourceMaps-nuxtHooks.test.ts +++ b/packages/nuxt/test/vite/sourceMaps-nuxtHooks.test.ts @@ -22,16 +22,16 @@ vi.mock('@sentry/bundler-plugins/vite', () => ({ sentryVitePlugin: mockSentryVit vi.mock('@sentry/bundler-plugins/rollup', () => ({ sentryRollupPlugin: mockSentryRollupPlugin })); function createMockAddVitePlugin() { - let capturedPlugins: Plugin[] | null = null; + const capturedPlugins: Plugin[] = []; const mockAddVitePlugin = vi.fn((plugins: Plugin[]) => { - capturedPlugins = plugins; + capturedPlugins.push(...plugins); }); return { mockAddVitePlugin, - getCapturedPlugin: () => capturedPlugins?.[0] ?? null, - getCapturedPlugins: () => capturedPlugins, + getValidationPlugin: () => + capturedPlugins.find(plugin => plugin.name === 'sentry-nuxt-source-map-validation') ?? null, }; } @@ -121,15 +121,30 @@ describe('setupSourceMaps hooks', () => { }); describe('vite plugin registration', () => { - it('calls `addVitePlugin` when setupSourceMaps is called', async () => { + it('adds the source map validation plugin to the client and server builds', async () => { const mockNuxt = createMockNuxt({ _prepare: false, dev: false }); - const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin(); + const { mockAddVitePlugin } = createMockAddVitePlugin(); setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin); - const plugin = getCapturedPlugin(); - expect(plugin).not.toBeNull(); - expect(plugin?.name).toBe('sentry-nuxt-source-map-validation'); + expect(mockAddVitePlugin).toHaveBeenCalledWith( + [expect.objectContaining({ name: 'sentry-nuxt-source-map-validation' })], + { dev: false, build: true }, + ); + }); + + it('adds the Sentry Vite plugin to the client build only', async () => { + const mockNuxt = createMockNuxt({ _prepare: false, dev: false }); + const { mockAddVitePlugin } = createMockAddVitePlugin(); + + setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin); + + expect(mockAddVitePlugin).toHaveBeenCalledTimes(2); + expect(mockAddVitePlugin).toHaveBeenCalledWith([{ name: 'sentry-vite-plugin' }], { + dev: false, + build: true, + server: false, + }); }); it.each([ @@ -160,21 +175,6 @@ describe('setupSourceMaps hooks', () => { expect(mockAddVitePlugin).not.toHaveBeenCalled(); }); - - it.each([ - { label: 'server (SSR) build', buildConfig: { build: { ssr: true }, plugins: [] } }, - { label: 'client build', buildConfig: { build: { ssr: false }, plugins: [] } }, - ])('adds sentry vite plugin to vite config for $label in production', async () => { - const mockNuxt = createMockNuxt({ _prepare: false, dev: false }); - const { mockAddVitePlugin, getCapturedPlugins } = createMockAddVitePlugin(); - - setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin); - - const plugins = getCapturedPlugins(); - expect(plugins).not.toBeNull(); - expect(plugins?.length).toBeGreaterThan(0); - expect(mockSentryVitePlugin).toHaveBeenCalled(); - }); }); describe('sentry vite plugin calls', () => { @@ -372,12 +372,12 @@ describe('setupSourceMaps hooks', () => { describe('debug logging', () => { it('logs a [Sentry] message in production mode', async () => { const mockNuxt = createMockNuxt({ _prepare: false, dev: false }); - const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin(); + const { mockAddVitePlugin, getValidationPlugin } = createMockAddVitePlugin(); setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin); await mockNuxt.triggerHook('modules:done'); - const plugin = getCapturedPlugin(); + const plugin = getValidationPlugin(); if (plugin && typeof plugin.config === 'function') { (plugin.config as (config: UserConfig, env: ConfigEnv) => void)( { build: { ssr: false }, plugins: [] } as UserConfig, @@ -398,12 +398,12 @@ describe('setupSourceMaps hooks', () => { it('does not log a [Sentry] messages in prepare mode', async () => { const mockNuxt = createMockNuxt({ _prepare: true }); - const { mockAddVitePlugin, getCapturedPlugin } = createMockAddVitePlugin(); + const { mockAddVitePlugin, getValidationPlugin } = createMockAddVitePlugin(); setupSourceMaps({ debug: true }, mockNuxt as unknown as Nuxt, mockAddVitePlugin); await mockNuxt.triggerHook('modules:done'); - const plugin = getCapturedPlugin(); + const plugin = getValidationPlugin(); if (plugin && typeof plugin.config === 'function') { (plugin.config as (config: UserConfig, env: ConfigEnv) => void)({ build: {}, plugins: [] } as UserConfig, { mode: 'production',