diff --git a/packages/nestjs/src/integrations/contextlines.ts b/packages/nestjs/src/integrations/contextlines.ts new file mode 100644 index 000000000000..c867c68f56c1 --- /dev/null +++ b/packages/nestjs/src/integrations/contextlines.ts @@ -0,0 +1,21 @@ +import type { IntegrationFn } from '@sentry/core'; +import { defineIntegration } from '@sentry/core'; +import { contextLinesIntegration as originalContextLinesIntegration } from '@sentry/node'; + +const _contextLinesIntegration = ((options?: { frameContextLines?: number }) => { + return originalContextLinesIntegration({ + ...options, + // Nest.js always enabled this, without an easy way for us to detect this + // so we just enable it by default + // see: https://github.com/nestjs/nest-cli/blob/f5dbb573df1fe103139026a36b6d0efe65e8e985/actions/start.action.ts#L220 + hasSourceMaps: true, + }); +}) satisfies IntegrationFn; + +/** + * Capture the lines before and after the frame's context. + * + * A Nest-specific variant of the node-core contextLineIntegration. + * This has source maps enabled by default, as Nest.js enables this under the hood. + */ +export const contextLinesIntegration = defineIntegration(_contextLinesIntegration); diff --git a/packages/nestjs/src/sdk.ts b/packages/nestjs/src/sdk.ts index 733cb935003c..5aea07d6e65f 100644 --- a/packages/nestjs/src/sdk.ts +++ b/packages/nestjs/src/sdk.ts @@ -7,6 +7,7 @@ import { } from '@sentry/core'; import type { NodeClient, NodeOptions, Span } from '@sentry/node'; import { getDefaultIntegrations as getDefaultNodeIntegrations, init as nodeInit } from '@sentry/node'; +import { contextLinesIntegration } from './integrations/contextlines'; import { nestIntegration } from './integrations/nest'; /** @@ -34,7 +35,12 @@ export function init(options: NodeOptions | undefined = {}): NodeClient | undefi /** Get the default integrations for the NestJS SDK. */ export function getDefaultIntegrations(options: NodeOptions): Integration[] | undefined { - return [nestIntegration(), ...getDefaultNodeIntegrations(options)]; + return [ + nestIntegration(), + ...getDefaultNodeIntegrations(options).filter(i => i.name !== 'ContextLines'), + // Custom variant for Nest.js + contextLinesIntegration(), + ]; } function addNestSpanAttributes(span: Span): void { diff --git a/packages/node-core/src/integrations/contextlines.ts b/packages/node-core/src/integrations/contextlines.ts index 5c99166d0d54..6402bbd1dd24 100644 --- a/packages/node-core/src/integrations/contextlines.ts +++ b/packages/node-core/src/integrations/contextlines.ts @@ -22,6 +22,14 @@ interface ContextLinesOptions { * Set to 0 to disable loading and inclusion of source files. **/ frameContextLines?: number; + + /** + * If error stacktraces are already sourcemapped. + * In this case, we can skip the sourcemap lookup for certain cases. + * This is the case e.g. if the node process is run with `node --enable-source-maps`. + * If this is undefined, the SDK tries to infer it from the environment. + */ + hasSourceMaps?: boolean; } /** @@ -62,6 +70,19 @@ function shouldSkipContextLinesForFile(path: string): boolean { return false; } +/** + * Skip frames that we determine to already have been sourcemapped. + */ +function shouldSkipContextLinesThatAreAlreadySourceMapped(path: string, frame: StackFrame): boolean { + // For non-in-app frames, we skip context lines when we are reasonably certain that the path is already sourcemapped. + // For now, we only consider .ts files because they can never appear otherwise in a stackframe, if not already sourcemapped. + if (frame.in_app === false && path.endsWith('.ts')) { + return true; + } + + return false; +} + /** * Determines if we should skip contextlines based off the max lineno and colno values. */ @@ -164,10 +185,11 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output: // We use this inside Promise.all, so we need to resolve the promise even if there is an error // to prevent Promise.all from short circuiting the rest. - function onStreamError(e: Error): void { + function onStreamError(): void { // Mark file path as failed to read and prevent multiple read attempts. LRU_FILE_CONTENTS_FS_READ_FAILED.set(path, 1); - DEBUG_BUILD && debug.error(`Failed to read file: ${path}. Error: ${e}`); + DEBUG_BUILD && + debug.warn(`ContextLines: Failed to read file: ${path}. Skipping context line extraction for this file.`); lineReaded.close(); lineReaded.removeAllListeners(); destroyStreamAndResolve(); @@ -215,7 +237,10 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output: * failing reads from happening. */ /* eslint-disable complexity */ -async function addSourceContext(event: Event, contextLines: number): Promise { +async function addSourceContext( + event: Event, + { contextLines, hasSourceMaps }: { contextLines: number; hasSourceMaps: boolean }, +): Promise { // keep a lookup map of which files we've already enqueued to read, // so we don't enqueue the same file multiple times which would cause multiple i/o reads const filesToLines: Record = {}; @@ -242,6 +267,10 @@ async function addSourceContext(event: Event, contextLines: number): Promise { const contextLines = options.frameContextLines !== undefined ? options.frameContextLines : DEFAULT_LINES_OF_CONTEXT; + const hasSourceMaps = options.hasSourceMaps ?? inferHasSourceMaps(); return { name: INTEGRATION_NAME, processEvent(event) { - return addSourceContext(event, contextLines); + return addSourceContext(event, { contextLines, hasSourceMaps }); }, }; }) satisfies IntegrationFn; @@ -412,3 +442,10 @@ export const _contextLinesIntegration = ((options: ContextLinesOptions = {}) => * Capture the lines before and after the frame's context. */ export const contextLinesIntegration = defineIntegration(_contextLinesIntegration); + +function inferHasSourceMaps(): boolean { + return ( + (process.env.NODE_OPTIONS?.includes('--enable-source-maps') || process.argv.includes('--enable-source-maps')) ?? + false + ); +} diff --git a/packages/node-core/test/integrations/contextlines.test.ts b/packages/node-core/test/integrations/contextlines.test.ts index 2965acded28f..9a0dc1d215e3 100644 --- a/packages/node-core/test/integrations/contextlines.test.ts +++ b/packages/node-core/test/integrations/contextlines.test.ts @@ -232,4 +232,114 @@ describe('ContextLines', () => { expect(readStreamSpy).not.toHaveBeenCalled(); }); }); + + describe('hasSourceMaps', () => { + let _argv: string[]; + let _env: any; + + beforeEach(() => { + _argv = process.argv; + _env = process.env; + }); + + afterEach(() => { + process.argv = _argv; + process.env = _env; + }); + + test.each([ + [undefined, 'file:///var/task/hasSourceMaps/index1a.ts', true], + [true, 'file:///var/task/hasSourceMaps/index1b.ts', true], + [false, 'file:///var/task/hasSourceMaps/index1c.ts', false], + [undefined, 'file:///var/task/hasSourceMaps/index1d.js', true], + [true, 'file:///var/task/hasSourceMaps/index1e.js', true], + [false, 'file:///var/task/hasSourceMaps/index1f.js', true], + ])('handles in_app=%s, filename=%s', async (in_app, filename, hasBeenCalled) => { + contextLines = _contextLinesIntegration({ hasSourceMaps: true }); + const readStreamSpy = vi.spyOn(fs, 'createReadStream'); + + const frames: StackFrame[] = [ + { + colno: 1, + filename, + lineno: 1, + function: 'fxn1', + in_app, + }, + ]; + + await addContext(frames); + + expect(readStreamSpy).toHaveBeenCalledTimes(hasBeenCalled ? 1 : 0); + }); + + test('infer hasSourceMaps from NODE_OPTIONS', async () => { + process.env = { + ..._env, + NODE_OPTIONS: '--enable-source-maps --other-option', + }; + + contextLines = _contextLinesIntegration(); + const readStreamSpy = vi.spyOn(fs, 'createReadStream'); + + const frames: StackFrame[] = [ + { + colno: 1, + filename: 'file:///var/task/hasSourceMaps/index-infer1.ts', + lineno: 1, + function: 'fxn1', + in_app: false, + }, + ]; + + await addContext(frames); + + expect(readStreamSpy).toHaveBeenCalledTimes(0); + }); + + test('infer hasSourceMaps from process.argv', async () => { + process.argv = [..._argv, '--enable-source-maps', '--other-option']; + + contextLines = _contextLinesIntegration(); + const readStreamSpy = vi.spyOn(fs, 'createReadStream'); + + const frames: StackFrame[] = [ + { + colno: 1, + filename: 'file:///var/task/hasSourceMaps/index-infer2.ts', + lineno: 1, + function: 'fxn1', + in_app: false, + }, + ]; + + await addContext(frames); + + expect(readStreamSpy).toHaveBeenCalledTimes(0); + }); + + test('does not infer hasSourceMaps if hasSourceMaps is set', async () => { + process.env = { + ..._env, + NODE_OPTIONS: '--enable-source-maps --other-option', + }; + + contextLines = _contextLinesIntegration({ hasSourceMaps: false }); + const readStreamSpy = vi.spyOn(fs, 'createReadStream'); + + const frames: StackFrame[] = [ + { + colno: 1, + filename: 'file:///var/task/hasSourceMaps/index-infer3.ts', + lineno: 1, + function: 'fxn1', + in_app: false, + }, + ]; + + await addContext(frames); + + expect(readStreamSpy).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/node-core/test/sdk/init.test.ts b/packages/node-core/test/sdk/init.test.ts index c0c574ab2b35..8fff1f6e6aa4 100644 --- a/packages/node-core/test/sdk/init.test.ts +++ b/packages/node-core/test/sdk/init.test.ts @@ -220,6 +220,11 @@ describe('init()', () => { }); describe('validateOpenTelemetrySetup', () => { + beforeEach(() => { + // just swallowing these + vi.spyOn(debug, 'log').mockImplementation(() => {}); + }); + afterEach(() => { global.__SENTRY__ = {}; cleanupOtel();