From 31a13929363c7422d33437333520934815be324e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 22 Mar 2022 19:54:52 +0000 Subject: [PATCH] Add context from LinkedErrors --- .../node/src/integrations/contextlines.ts | 29 +++++++++++-------- .../node/src/integrations/linkederrors.ts | 14 +++++++-- packages/node/src/sdk.ts | 3 +- packages/node/test/context-lines.test.ts | 8 +++-- packages/node/test/index.test.ts | 6 ++-- 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index 3fe47a3de25c..e18768b2fe3d 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/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 { - if (contextLines > 0 && event.exception?.values) { + public async addSourceContext(event: Event): Promise { + 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 { + public async addSourceContextToFrames(frames: StackFrame[]): Promise { + 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) { diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index 854b47a6281f..701400594971 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/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 { + private async _walkErrorTree(error: ExtendedError, key: string, stack: Exception[] = []): Promise { 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((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((resolve, reject) => { void this._walkErrorTree(error[key], key, [exception, ...stack]) .then(resolve) .then(null, () => { diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index f944aedd5b92..201b4b073f86 100644 --- a/packages/node/src/sdk.ts +++ b/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(), ]; /** diff --git a/packages/node/test/context-lines.test.ts b/packages/node/test/context-lines.test.ts index 1182e63360c6..821c95d7b6b0 100644 --- a/packages/node/test/context-lines.test.ts +++ b/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 { - await contextLines.addSourceContext({ exception: { values: [{ stacktrace: { frames } }] } }, lines); + async function addContext(frames: StackFrame[]): Promise { + 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); }); }); diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 91ce4945fb21..0af38dae80fb 100644 --- a/packages/node/test/index.test.ts +++ b/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();