Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Temporarily store debug IDs in stack frame and only put them into debug_meta before sending #8347

Merged
merged 3 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 41 additions & 8 deletions packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -75,6 +75,11 @@ export function prepareEvent(
}

return result.then(evt => {
// TODO explain why this is here
lforst marked this conversation as resolved.
Show resolved Hide resolved
if (evt) {
applyDebugMeta(evt);
}

if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth);
}
Expand Down Expand Up @@ -121,9 +126,9 @@ function applyClientOptions(event: Event, options: ClientOptions): void {
const debugIdStackParserCache = new WeakMap<StackParser, Map<string, StackFrame[]>>();

/**
* 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) {
Expand Down Expand Up @@ -160,27 +165,55 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void
return acc;
}, {});

// Get a Set of filenames in the stack trace
const errorFileNames = new Set<string>();
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<string, string> = {};
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;
}
});
});
} catch (e) {
// 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 => {
Object.keys(filenameDebugIdMap).forEach(filename => {
if (filenameDebugIdMap[filename]) {
lforst marked this conversation as resolved.
Show resolved Hide resolved
images.push({
type: 'sourcemap',
Expand Down
71 changes: 55 additions & 16 deletions packages/core/test/lib/prepareEvent.test.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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',
});
});
});