ref(node): Add source code context when using LinkedErrors (#4753)

- Modifies the `ContextLines` integration so it can be used from elsewhere with it's options
- In the `LinkedErrors` integration, if the  `ContextLines` integration is enabled, adds context to linked errors

Author: timfish

packages/node/src/integrations/contextlines.ts

@@ -51,10 +51,8 @@ export class ContextLines implements Integration {
 
   public constructor(private readonly _options: ContextLinesOptions = {}) {}
 
-  /**
-   * @inheritDoc
-   */
-  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
+  /** Get's the number of context lines to add */
+  private get _contextLines(): number {
     // This is only here to copy frameContextLines from init options if it hasn't
     // been set via this integrations constructor.
     //
@@ -65,18 +63,22 @@ export class ContextLines implements Integration {
       this._options.frameContextLines = initOptions?.frameContextLines;
     }
 
-    const contextLines =
-      this._options.frameContextLines !== undefined ? this._options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;
+    return this._options.frameContextLines !== undefined ? this._options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;
+  }
 
-    addGlobalEventProcessor(event => this.addSourceContext(event, contextLines));
+  /**
+   * @inheritDoc
+   */
+  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
+    addGlobalEventProcessor(event => this.addSourceContext(event));
   }
 
   /** Processes an event and adds context lines */
-  public async addSourceContext(event: Event, contextLines: number): Promise<Event> {
-    if (contextLines > 0 && event.exception?.values) {
+  public async addSourceContext(event: Event): Promise<Event> {
+    if (this._contextLines > 0 && event.exception?.values) {
       for (const exception of event.exception.values) {
         if (exception.stacktrace?.frames) {
-          await this._addSourceContextToFrames(exception.stacktrace.frames, contextLines);
+          await this.addSourceContextToFrames(exception.stacktrace.frames);
         }
       }
     }
@@ -85,9 +87,12 @@ export class ContextLines implements Integration {
   }
 
   /** Adds context lines to frames */
-  public async _addSourceContextToFrames(frames: StackFrame[], contextLines: number): Promise<void> {
+  public async addSourceContextToFrames(frames: StackFrame[]): Promise<void> {
+    const contextLines = this._contextLines;
+
     for (const frame of frames) {
-      if (frame.filename) {
+      // Only add context if we have a filename and it hasn't already been added
+      if (frame.filename && frame.context_line === undefined) {
         const sourceFile = await _readSourceFile(frame.filename);
 
         if (sourceFile) {

packages/node/src/integrations/linkederrors.ts

@@ -3,6 +3,7 @@ import { Event, EventHint, Exception, ExtendedError, Integration } from '@sentry
 import { isInstanceOf, resolvedSyncPromise, SyncPromise } from '@sentry/utils';
 
 import { exceptionFromError } from '../eventbuilder';
+import { ContextLines } from './contextlines';
 
 const DEFAULT_KEY = 'cause';
 const DEFAULT_LIMIT = 5;
@@ -76,14 +77,21 @@ export class LinkedErrors implements Integration {
   /**
    * @inheritDoc
    */
-  private _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): PromiseLike<Exception[]> {
+  private async _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): Promise<Exception[]> {
     if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) {
-      return resolvedSyncPromise(stack);
+      return Promise.resolve(stack);
     }
 
     const exception = exceptionFromError(error[key]);
 
-    return new SyncPromise<Exception[]>((resolve, reject) => {
+    // If the ContextLines integration is enabled, we add source code context to linked errors
+    // because we can't guarantee the order that integrations are run.
+    const contextLines = getCurrentHub().getIntegration(ContextLines);
+    if (contextLines && exception.stacktrace?.frames) {
+      await contextLines.addSourceContextToFrames(exception.stacktrace.frames);
+    }
+
+    return new Promise<Exception[]>((resolve, reject) => {
       void this._walkErrorTree(error[key], key, [exception, ...stack])
         .then(resolve)
         .then(null, () => {

packages/node/src/sdk.ts

@@ -12,6 +12,7 @@ export const defaultIntegrations = [
   // Common
   new CoreIntegrations.InboundFilters(),
   new CoreIntegrations.FunctionToString(),
+  new ContextLines(),
   // Native Wrappers
   new Console(),
   new Http(),
@@ -20,8 +21,6 @@ export const defaultIntegrations = [
   new OnUnhandledRejection(),
   // Misc
   new LinkedErrors(),
-  // ContextLines must come after LinkedErrors so that context is added to linked errors
-  new ContextLines(),
 ];
 
 /**

packages/node/test/context-lines.test.ts

@@ -9,8 +9,8 @@ describe('ContextLines', () => {
   let readFileSpy: jest.SpyInstance;
   let contextLines: ContextLines;
 
-  async function addContext(frames: StackFrame[], lines: number = 7): Promise<void> {
-    await contextLines.addSourceContext({ exception: { values: [{ stacktrace: { frames } }] } }, lines);
+  async function addContext(frames: StackFrame[]): Promise<void> {
+    await contextLines.addSourceContext({ exception: { values: [{ stacktrace: { frames } }] } });
   }
 
   beforeEach(() => {
@@ -97,10 +97,12 @@ describe('ContextLines', () => {
     });
 
     test('parseStack with no context', async () => {
+      contextLines = new ContextLines({ frameContextLines: 0 });
+
       expect.assertions(1);
       const frames = parseStackFrames(new Error('test'));
 
-      await addContext(frames, 0);
+      await addContext(frames);
       expect(readFileSpy).toHaveBeenCalledTimes(0);
     });
   });

packages/node/test/index.test.ts

@@ -16,7 +16,7 @@ import {
   Scope,
 } from '../src';
 import { NodeBackend } from '../src/backend';
-import { LinkedErrors } from '../src/integrations';
+import { ContextLines, LinkedErrors } from '../src/integrations';
 
 jest.mock('@sentry/core', () => {
   const original = jest.requireActual('@sentry/core');
@@ -199,11 +199,11 @@ describe('SentryNode', () => {
       }
     });
 
-    test.only('capture a linked exception with pre/post context', done => {
+    test('capture a linked exception with pre/post context', done => {
       expect.assertions(15);
       getCurrentHub().bindClient(
         new NodeClient({
-          integrations: i => [new LinkedErrors(), ...i],
+          integrations: [new ContextLines(), new LinkedErrors()],
           beforeSend: (event: Event) => {
             expect(event.exception).not.toBeUndefined();
             expect(event.exception!.values![1]).not.toBeUndefined();