feat(node): Skip context lines lookup when source maps are enabled

Author: mydea

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);

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 {

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<Event> {
+async function addSourceContext(
+  event: Event,
+  { contextLines, hasSourceMaps }: { contextLines: number; hasSourceMaps: boolean },
+): Promise<Event> {
   // 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<string, number[]> = {};
@@ -242,6 +267,10 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
           continue;
         }
 
+        if (hasSourceMaps && shouldSkipContextLinesThatAreAlreadySourceMapped(filename, frame)) {
+          continue;
+        }
+
         const filesToLinesOutput = filesToLines[filename];
         if (!filesToLinesOutput) filesToLines[filename] = [];
         // @ts-expect-error this is defined above
@@ -399,11 +428,12 @@ function makeContextRange(line: number, linecontext: number): [start: number, en
 /** Exported only for tests, as a type-safe variant. */
 export const _contextLinesIntegration = ((options: ContextLinesOptions = {}) => {
   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
+  );
+}

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);
+    });
+  });
 });

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();