From b005d08b6f36f43edad99f9a4c40cee1afafbcd5 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 30 Jun 2023 14:50:28 +0200 Subject: [PATCH 1/6] feat(core): Add functions to get module metadata from injected code --- packages/core/src/index.ts | 1 + packages/core/src/metadata.ts | 87 ++++++++++++++++++++ packages/core/test/lib/metadata.test.ts | 102 ++++++++++++++++++++++++ packages/types/src/stackframe.ts | 1 + packages/utils/src/worldwide.ts | 13 +++ 5 files changed, 204 insertions(+) create mode 100644 packages/core/src/metadata.ts create mode 100644 packages/core/test/lib/metadata.test.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index db9d21c2f2e8..fcc37f17c989 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -46,6 +46,7 @@ export { prepareEvent } from './utils/prepareEvent'; export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { DEFAULT_ENVIRONMENT } from './constants'; +export { getMetadataForUrl, addMetadataToStackFrames, stripMetadataFromStackFrames } from './metadata'; import * as Integrations from './integrations'; diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts new file mode 100644 index 000000000000..77e2f3c388d5 --- /dev/null +++ b/packages/core/src/metadata.ts @@ -0,0 +1,87 @@ +import type { Event, StackFrame, StackParser } from '@sentry/types'; +import { GLOBAL_OBJ } from '@sentry/utils'; + +function ensureMetadataStacksAreParsed(parser: StackParser): void { + if (!GLOBAL_OBJ.__MODULE_METADATA__) { + return; + } + + for (const stack of Object.keys(GLOBAL_OBJ.__MODULE_METADATA__)) { + const metadata = GLOBAL_OBJ.__MODULE_METADATA__[stack]; + + // If this stack has already been parsed, skip it + if (metadata == false) { + continue; + } + + // Ensure this stack doesn't get parsed again + GLOBAL_OBJ.__MODULE_METADATA__[stack] = false; + + const frames = parser(stack); + + // Go through the frames starting from the top of the stack and find the first one with a filename + for (const frame of frames.reverse()) { + if (frame.filename) { + // Save the metadata for this filename + GLOBAL_OBJ.__MODULE_METADATA_PARSED__ = GLOBAL_OBJ.__MODULE_METADATA_PARSED__ || {}; + GLOBAL_OBJ.__MODULE_METADATA_PARSED__[frame.filename] = metadata; + break; + } + } + } +} + +/** + * Retrieve metadata for a specific JavaScript file URL. + * + * Metadata is injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. + */ +export function getMetadataForUrl(parser: StackParser, url: string): object | undefined { + ensureMetadataStacksAreParsed(parser); + + const metadataObj = GLOBAL_OBJ.__MODULE_METADATA_PARSED__ || {}; + + return metadataObj[url]; +} + +/** + * Adds metadata to stack frames. + * + * Metadata is injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. + */ +export function addMetadataToStackFrames(parser: StackParser, event: Event): void { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + event.exception!.values!.forEach(exception => { + if (!exception.stacktrace) { + return; + } + + for (const frame of exception.stacktrace.frames || []) { + if (!frame.filename) { + continue; + } + + const metadata = getMetadataForUrl(parser, frame.filename); + + if (metadata) { + frame.module_metadata = metadata; + } + } + }); +} + +/** + * Strips metadata from stack frames. + */ +export function stripMetadataFromStackFrames(event: Event): void { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + event.exception!.values!.forEach(exception => { + if (!exception.stacktrace) { + return; + } + + for (const frame of exception.stacktrace.frames || []) { + delete frame.module_metadata; + } + }); +} diff --git a/packages/core/test/lib/metadata.test.ts b/packages/core/test/lib/metadata.test.ts new file mode 100644 index 000000000000..60fb8d5224b4 --- /dev/null +++ b/packages/core/test/lib/metadata.test.ts @@ -0,0 +1,102 @@ +import type { Event } from '@sentry/types'; +import { createStackParser, GLOBAL_OBJ, nodeStackLineParser } from '@sentry/utils'; + +import { addMetadataToStackFrames, getMetadataForUrl, stripMetadataFromStackFrames } from '../../src'; + +const parser = createStackParser(nodeStackLineParser()); + +const stack = new Error().stack || ''; + +const event: Event = { + exception: { + values: [ + { + stacktrace: { + frames: [ + { + filename: '', + function: 'new Promise', + }, + { + filename: '/tmp/utils.js', + function: 'Promise.then.completed', + lineno: 391, + colno: 28, + }, + { + filename: __filename, + function: 'Object.', + lineno: 9, + colno: 19, + }, + ], + }, + }, + ], + }, +}; + +describe('Metadata', () => { + beforeEach(() => { + GLOBAL_OBJ.__MODULE_METADATA__ = GLOBAL_OBJ.__MODULE_METADATA__ || {}; + GLOBAL_OBJ.__MODULE_METADATA__[stack] = { team: 'frontend' }; + }); + + it('is parsed', () => { + const metadata = getMetadataForUrl(parser, __filename); + + expect(metadata).toEqual({ team: 'frontend' }); + // should now be false so it doesn't get parsed again + expect(GLOBAL_OBJ.__MODULE_METADATA__?.[stack]).toBe(false); + }); + + it('is added and stripped from stack frames', () => { + addMetadataToStackFrames(parser, event); + + expect(event.exception?.values?.[0].stacktrace?.frames).toEqual([ + { + filename: '', + function: 'new Promise', + }, + { + filename: '/tmp/utils.js', + function: 'Promise.then.completed', + lineno: 391, + colno: 28, + }, + { + filename: __filename, + function: 'Object.', + lineno: 9, + colno: 19, + module_metadata: { + team: 'frontend', + }, + }, + ]); + + // should now be false so it doesn't get parsed again + expect(GLOBAL_OBJ.__MODULE_METADATA__?.[stack]).toBe(false); + + stripMetadataFromStackFrames(event); + + expect(event.exception?.values?.[0].stacktrace?.frames).toEqual([ + { + filename: '', + function: 'new Promise', + }, + { + filename: '/tmp/utils.js', + function: 'Promise.then.completed', + lineno: 391, + colno: 28, + }, + { + filename: __filename, + function: 'Object.', + lineno: 9, + colno: 19, + }, + ]); + }); +}); diff --git a/packages/types/src/stackframe.ts b/packages/types/src/stackframe.ts index 7a8058a1792c..6459cd569f46 100644 --- a/packages/types/src/stackframe.ts +++ b/packages/types/src/stackframe.ts @@ -15,4 +15,5 @@ export interface StackFrame { addr_mode?: string; vars?: { [key: string]: any }; debug_id?: string; + module_metadata?: object; } diff --git a/packages/utils/src/worldwide.ts b/packages/utils/src/worldwide.ts index 37a6deb30851..ab9e696ec0ef 100644 --- a/packages/utils/src/worldwide.ts +++ b/packages/utils/src/worldwide.ts @@ -55,6 +55,19 @@ export interface InternalGlobal { [key: string]: Function; }; }; + /** + * Raw module metadata that is injected by bundler plugins. + * + * Keys are `error.stack` strings, values are metadata objects. + * Once the stack has been parsed, the value is set to `false` so they do not get parsed multiple times. + */ + __MODULE_METADATA__?: Record; + /** + * Parsed module metadata. + * + * Keys are source filenames/urls, values are metadata objects. + */ + __MODULE_METADATA_PARSED__?: Record; } // The code below for 'isGlobalObj' and 'GLOBAL_OBJ' was copied from core-js before modification From a6755b56f39692824952ba21b9320af102ddf386 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 30 Jun 2023 15:12:13 +0200 Subject: [PATCH 2/6] Fix linting --- packages/core/src/metadata.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 77e2f3c388d5..f20c4ae4b5fd 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -1,4 +1,4 @@ -import type { Event, StackFrame, StackParser } from '@sentry/types'; +import type { Event, StackParser } from '@sentry/types'; import { GLOBAL_OBJ } from '@sentry/utils'; function ensureMetadataStacksAreParsed(parser: StackParser): void { From 382f593c00d1957f276b693555f45d73d7c173fe Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 30 Jun 2023 16:06:10 +0200 Subject: [PATCH 3/6] We don't need to two globals! --- packages/core/src/metadata.ts | 16 +++++++++------- packages/utils/src/worldwide.ts | 6 ------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index f20c4ae4b5fd..5c916b522d55 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -1,6 +1,11 @@ import type { Event, StackParser } from '@sentry/types'; import { GLOBAL_OBJ } from '@sentry/utils'; +/** + * Keys are source filenames/urls, values are metadata objects. + */ +let filenameMetadataMap: Record | undefined; + function ensureMetadataStacksAreParsed(parser: StackParser): void { if (!GLOBAL_OBJ.__MODULE_METADATA__) { return; @@ -9,7 +14,7 @@ function ensureMetadataStacksAreParsed(parser: StackParser): void { for (const stack of Object.keys(GLOBAL_OBJ.__MODULE_METADATA__)) { const metadata = GLOBAL_OBJ.__MODULE_METADATA__[stack]; - // If this stack has already been parsed, skip it + // If this stack has already been parsed, we can skip it if (metadata == false) { continue; } @@ -22,9 +27,9 @@ function ensureMetadataStacksAreParsed(parser: StackParser): void { // Go through the frames starting from the top of the stack and find the first one with a filename for (const frame of frames.reverse()) { if (frame.filename) { + filenameMetadataMap = filenameMetadataMap || {}; // Save the metadata for this filename - GLOBAL_OBJ.__MODULE_METADATA_PARSED__ = GLOBAL_OBJ.__MODULE_METADATA_PARSED__ || {}; - GLOBAL_OBJ.__MODULE_METADATA_PARSED__[frame.filename] = metadata; + filenameMetadataMap[frame.filename] = metadata; break; } } @@ -38,10 +43,7 @@ function ensureMetadataStacksAreParsed(parser: StackParser): void { */ export function getMetadataForUrl(parser: StackParser, url: string): object | undefined { ensureMetadataStacksAreParsed(parser); - - const metadataObj = GLOBAL_OBJ.__MODULE_METADATA_PARSED__ || {}; - - return metadataObj[url]; + return filenameMetadataMap ? filenameMetadataMap[url] : undefined; } /** diff --git a/packages/utils/src/worldwide.ts b/packages/utils/src/worldwide.ts index ab9e696ec0ef..2d2c9d75982c 100644 --- a/packages/utils/src/worldwide.ts +++ b/packages/utils/src/worldwide.ts @@ -62,12 +62,6 @@ export interface InternalGlobal { * Once the stack has been parsed, the value is set to `false` so they do not get parsed multiple times. */ __MODULE_METADATA__?: Record; - /** - * Parsed module metadata. - * - * Keys are source filenames/urls, values are metadata objects. - */ - __MODULE_METADATA_PARSED__?: Record; } // The code below for 'isGlobalObj' and 'GLOBAL_OBJ' was copied from core-js before modification From 8707a8c959378d8c5c0d437dae5742ebbb9c3392 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 3 Jul 2023 15:11:36 +0200 Subject: [PATCH 4/6] PR review changes --- packages/core/src/index.ts | 1 - packages/core/src/metadata.ts | 28 ++++++++++++------------- packages/core/test/lib/metadata.test.ts | 11 +++------- packages/utils/src/worldwide.ts | 5 ++--- 4 files changed, 19 insertions(+), 26 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index fcc37f17c989..db9d21c2f2e8 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -46,7 +46,6 @@ export { prepareEvent } from './utils/prepareEvent'; export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { DEFAULT_ENVIRONMENT } from './constants'; -export { getMetadataForUrl, addMetadataToStackFrames, stripMetadataFromStackFrames } from './metadata'; import * as Integrations from './integrations'; diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 5c916b522d55..972c7b4c6124 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -1,35 +1,34 @@ import type { Event, StackParser } from '@sentry/types'; import { GLOBAL_OBJ } from '@sentry/utils'; -/** - * Keys are source filenames/urls, values are metadata objects. - */ -let filenameMetadataMap: Record | undefined; +/** Keys are source filename/url, values are metadata objects. */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const filenameMetadataMap = new Map(); +/** Set of stack strings that have already been parsed. */ +const parsedStacks = new Set(); function ensureMetadataStacksAreParsed(parser: StackParser): void { - if (!GLOBAL_OBJ.__MODULE_METADATA__) { + if (!GLOBAL_OBJ._sentryModuleMetadata) { return; } - for (const stack of Object.keys(GLOBAL_OBJ.__MODULE_METADATA__)) { - const metadata = GLOBAL_OBJ.__MODULE_METADATA__[stack]; + for (const stack of Object.keys(GLOBAL_OBJ._sentryModuleMetadata)) { + const metadata = GLOBAL_OBJ._sentryModuleMetadata[stack]; - // If this stack has already been parsed, we can skip it - if (metadata == false) { + if (parsedStacks.has(stack)) { continue; } // Ensure this stack doesn't get parsed again - GLOBAL_OBJ.__MODULE_METADATA__[stack] = false; + parsedStacks.add(stack); const frames = parser(stack); // Go through the frames starting from the top of the stack and find the first one with a filename for (const frame of frames.reverse()) { if (frame.filename) { - filenameMetadataMap = filenameMetadataMap || {}; // Save the metadata for this filename - filenameMetadataMap[frame.filename] = metadata; + filenameMetadataMap.set(frame.filename, metadata); break; } } @@ -41,9 +40,10 @@ function ensureMetadataStacksAreParsed(parser: StackParser): void { * * Metadata is injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. */ -export function getMetadataForUrl(parser: StackParser, url: string): object | undefined { +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function getMetadataForUrl(parser: StackParser, filename: string): any | undefined { ensureMetadataStacksAreParsed(parser); - return filenameMetadataMap ? filenameMetadataMap[url] : undefined; + return filenameMetadataMap.get(filename); } /** diff --git a/packages/core/test/lib/metadata.test.ts b/packages/core/test/lib/metadata.test.ts index 60fb8d5224b4..64b3fd7310ad 100644 --- a/packages/core/test/lib/metadata.test.ts +++ b/packages/core/test/lib/metadata.test.ts @@ -1,7 +1,7 @@ import type { Event } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ, nodeStackLineParser } from '@sentry/utils'; -import { addMetadataToStackFrames, getMetadataForUrl, stripMetadataFromStackFrames } from '../../src'; +import { addMetadataToStackFrames, getMetadataForUrl, stripMetadataFromStackFrames } from '../../src/metadata'; const parser = createStackParser(nodeStackLineParser()); @@ -38,16 +38,14 @@ const event: Event = { describe('Metadata', () => { beforeEach(() => { - GLOBAL_OBJ.__MODULE_METADATA__ = GLOBAL_OBJ.__MODULE_METADATA__ || {}; - GLOBAL_OBJ.__MODULE_METADATA__[stack] = { team: 'frontend' }; + GLOBAL_OBJ._sentryModuleMetadata = GLOBAL_OBJ._sentryModuleMetadata || {}; + GLOBAL_OBJ._sentryModuleMetadata[stack] = { team: 'frontend' }; }); it('is parsed', () => { const metadata = getMetadataForUrl(parser, __filename); expect(metadata).toEqual({ team: 'frontend' }); - // should now be false so it doesn't get parsed again - expect(GLOBAL_OBJ.__MODULE_METADATA__?.[stack]).toBe(false); }); it('is added and stripped from stack frames', () => { @@ -75,9 +73,6 @@ describe('Metadata', () => { }, ]); - // should now be false so it doesn't get parsed again - expect(GLOBAL_OBJ.__MODULE_METADATA__?.[stack]).toBe(false); - stripMetadataFromStackFrames(event); expect(event.exception?.values?.[0].stacktrace?.frames).toEqual([ diff --git a/packages/utils/src/worldwide.ts b/packages/utils/src/worldwide.ts index 2d2c9d75982c..12e098bf3bc7 100644 --- a/packages/utils/src/worldwide.ts +++ b/packages/utils/src/worldwide.ts @@ -58,10 +58,9 @@ export interface InternalGlobal { /** * Raw module metadata that is injected by bundler plugins. * - * Keys are `error.stack` strings, values are metadata objects. - * Once the stack has been parsed, the value is set to `false` so they do not get parsed multiple times. + * Keys are `error.stack` strings, values are the metadata. */ - __MODULE_METADATA__?: Record; + _sentryModuleMetadata?: Record; } // The code below for 'isGlobalObj' and 'GLOBAL_OBJ' was copied from core-js before modification From fde586518e0c918a1c2527ae910176a816618873 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 3 Jul 2023 15:45:58 +0200 Subject: [PATCH 5/6] Try catching! --- packages/core/src/metadata.ts | 54 ++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 972c7b4c6124..d1ebac6e90e5 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -52,38 +52,46 @@ export function getMetadataForUrl(parser: StackParser, filename: string): any | * Metadata is injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. */ export function addMetadataToStackFrames(parser: StackParser, event: Event): void { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - event.exception!.values!.forEach(exception => { - if (!exception.stacktrace) { - return; - } - - for (const frame of exception.stacktrace.frames || []) { - if (!frame.filename) { - continue; + try { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + event.exception!.values!.forEach(exception => { + if (!exception.stacktrace) { + return; } - const metadata = getMetadataForUrl(parser, frame.filename); + for (const frame of exception.stacktrace.frames || []) { + if (!frame.filename) { + continue; + } + + const metadata = getMetadataForUrl(parser, frame.filename); - if (metadata) { - frame.module_metadata = metadata; + if (metadata) { + frame.module_metadata = metadata; + } } - } - }); + }); + } catch (_) { + // To save bundle size we're just try catching here instead of checking for the existence of all the different objects. + } } /** * Strips metadata from stack frames. */ export function stripMetadataFromStackFrames(event: Event): void { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - event.exception!.values!.forEach(exception => { - if (!exception.stacktrace) { - return; - } + try { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + event.exception!.values!.forEach(exception => { + if (!exception.stacktrace) { + return; + } - for (const frame of exception.stacktrace.frames || []) { - delete frame.module_metadata; - } - }); + for (const frame of exception.stacktrace.frames || []) { + delete frame.module_metadata; + } + }); + } catch (_) { + // To save bundle size we're just try catching here instead of checking for the existence of all the different objects. + } } From 0748186d0e93be81d1167e0873a3e83d47c0e26c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 4 Jul 2023 13:14:22 +0200 Subject: [PATCH 6/6] `module_metadata` type --- packages/types/src/stackframe.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/types/src/stackframe.ts b/packages/types/src/stackframe.ts index 6459cd569f46..4f99f1ba3595 100644 --- a/packages/types/src/stackframe.ts +++ b/packages/types/src/stackframe.ts @@ -15,5 +15,5 @@ export interface StackFrame { addr_mode?: string; vars?: { [key: string]: any }; debug_id?: string; - module_metadata?: object; + module_metadata?: any; }