diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 474a95c7823b..84bfd404c56f 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -38,9 +38,9 @@ export function prepareEvent( applyClientOptions(prepared, options); applyIntegrationsMetadata(prepared, integrations); - // Only apply debug metadata to error events. + // Only put debug IDs onto frames for error events. if (event.type === undefined) { - applyDebugMetadata(prepared, options.stackParser); + applyDebugIds(prepared, options.stackParser); } // If we have scope given to us, use it as the base for further modifications. @@ -75,6 +75,14 @@ export function prepareEvent( } return result.then(evt => { + if (evt) { + // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified + // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed. + // This should not cause any PII issues, since we're only moving data that is already on the event and not adding + // any new data + applyDebugMeta(evt); + } + if (typeof normalizeDepth === 'number' && normalizeDepth > 0) { return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth); } @@ -121,9 +129,9 @@ function applyClientOptions(event: Event, options: ClientOptions): void { const debugIdStackParserCache = new WeakMap>(); /** - * Applies debug metadata images to the event in order to apply source maps by looking up their debug ID. + * Puts debug IDs into the stack frames of an error event. */ -export function applyDebugMetadata(event: Event, stackParser: StackParser): void { +export function applyDebugIds(event: Event, stackParser: StackParser): void { const debugIdMap = GLOBAL_OBJ._sentryDebugIds; if (!debugIdMap) { @@ -160,15 +168,39 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void return acc; }, {}); - // Get a Set of filenames in the stack trace - const errorFileNames = new Set(); try { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion event!.exception!.values!.forEach(exception => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion exception.stacktrace!.frames!.forEach(frame => { if (frame.filename) { - errorFileNames.add(frame.filename); + frame.debug_id = filenameDebugIdMap[frame.filename]; + } + }); + }); + } catch (e) { + // To save bundle size we're just try catching here instead of checking for the existence of all the different objects. + } +} + +/** + * Moves debug IDs from the stack frames of an error event into the debug_meta field. + */ +export function applyDebugMeta(event: Event): void { + // Extract debug IDs and filenames from the stack frames on the event. + const filenameDebugIdMap: Record = {}; + try { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + event.exception!.values!.forEach(exception => { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + exception.stacktrace!.frames!.forEach(frame => { + if (frame.debug_id) { + if (frame.abs_path) { + filenameDebugIdMap[frame.abs_path] = frame.debug_id; + } else if (frame.filename) { + filenameDebugIdMap[frame.filename] = frame.debug_id; + } + delete frame.debug_id; } }); }); @@ -176,18 +208,20 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void // To save bundle size we're just try catching here instead of checking for the existence of all the different objects. } + if (Object.keys(filenameDebugIdMap).length === 0) { + return; + } + // Fill debug_meta information event.debug_meta = event.debug_meta || {}; event.debug_meta.images = event.debug_meta.images || []; const images = event.debug_meta.images; - errorFileNames.forEach(filename => { - if (filenameDebugIdMap[filename]) { - images.push({ - type: 'sourcemap', - code_file: filename, - debug_id: filenameDebugIdMap[filename], - }); - } + Object.keys(filenameDebugIdMap).forEach(filename => { + images.push({ + type: 'sourcemap', + code_file: filename, + debug_id: filenameDebugIdMap[filename], + }); }); } diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index d08b4ace221d..1e8b53e60f8c 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -1,14 +1,14 @@ import type { Event } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ } from '@sentry/utils'; -import { applyDebugMetadata } from '../../src/utils/prepareEvent'; +import { applyDebugIds, applyDebugMeta } from '../../src/utils/prepareEvent'; -describe('applyDebugMetadata', () => { +describe('applyDebugIds', () => { afterEach(() => { GLOBAL_OBJ._sentryDebugIds = undefined; }); - it('should put debug source map images in debug_meta field', () => { + it("should put debug IDs into an event's stack frames", () => { GLOBAL_OBJ._sentryDebugIds = { 'filename1.js\nfilename1.js': 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa', 'filename2.js\nfilename2.js': 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb', @@ -34,35 +34,74 @@ describe('applyDebugMetadata', () => { }, }; - applyDebugMetadata(event, stackParser); + applyDebugIds(event, stackParser); - expect(event.debug_meta?.images).toContainEqual({ - type: 'sourcemap', - code_file: 'filename1.js', + expect(event.exception?.values?.[0].stacktrace?.frames).toContainEqual({ + filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa', }); - expect(event.debug_meta?.images).toContainEqual({ - type: 'sourcemap', - code_file: 'filename2.js', + expect(event.exception?.values?.[0].stacktrace?.frames).toContainEqual({ + filename: 'filename2.js', debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb', }); // expect not to contain an image for the stack frame that doesn't have a corresponding debug id - expect(event.debug_meta?.images).not.toContainEqual( + expect(event.exception?.values?.[0].stacktrace?.frames).not.toContainEqual( expect.objectContaining({ - type: 'sourcemap', - code_file: 'filename3.js', + filename3: 'filename3.js', + debug_id: expect.any(String), }), ); // expect not to contain an image for the debug id mapping that isn't contained in the stack trace - expect(event.debug_meta?.images).not.toContainEqual( + expect(event.exception?.values?.[0].stacktrace?.frames).not.toContainEqual( expect.objectContaining({ - type: 'sourcemap', - code_file: 'filename4.js', + filename3: 'filename4.js', debug_id: 'cccccccc-cccc-4ccc-cccc-cccccccccc', }), ); }); }); + +describe('applyDebugMeta', () => { + it("should move the debug IDs inside an event's stack frame into the debug_meta field", () => { + const event: Event = { + exception: { + values: [ + { + stacktrace: { + frames: [ + { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }, + { filename: 'filename2.js', debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb' }, + { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }, + { filename: 'filename3.js' }, + ], + }, + }, + ], + }, + }; + + applyDebugMeta(event); + + expect(event.exception?.values?.[0].stacktrace?.frames).toEqual([ + { filename: 'filename1.js' }, + { filename: 'filename2.js' }, + { filename: 'filename1.js' }, + { filename: 'filename3.js' }, + ]); + + expect(event.debug_meta?.images).toContainEqual({ + type: 'sourcemap', + code_file: 'filename1.js', + debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa', + }); + + expect(event.debug_meta?.images).toContainEqual({ + type: 'sourcemap', + code_file: 'filename2.js', + debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb', + }); + }); +});