From c55943f1d138876e83859f524b09e3c7fc8d6224 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 5 Jul 2023 11:59:24 +0200 Subject: [PATCH] fix(node): Apply source context to linked errors even when it is uncached (#8453) --- .../node/src/integrations/linkederrors.ts | 57 +++----- .../test/integrations/linkederrors.test.ts | 126 ++++++++---------- 2 files changed, 77 insertions(+), 106 deletions(-) diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index e2894a128be5..1283d2ebc1e1 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/packages/node/src/integrations/linkederrors.ts @@ -1,8 +1,7 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import type { Event, EventHint, Exception, ExtendedError, Integration, StackParser } from '@sentry/types'; -import { isInstanceOf, resolvedSyncPromise, SyncPromise } from '@sentry/utils'; +import { isInstanceOf } from '@sentry/utils'; -import type { NodeClient } from '../client'; import { exceptionFromError } from '../eventbuilder'; import { ContextLines } from './contextlines'; @@ -46,10 +45,18 @@ export class LinkedErrors implements Integration { addGlobalEventProcessor(async (event: Event, hint: EventHint) => { const hub = getCurrentHub(); const self = hub.getIntegration(LinkedErrors); - const client = hub.getClient(); + const client = hub.getClient(); if (client && self && self._handler && typeof self._handler === 'function') { - await self._handler(client.getOptions().stackParser, event, hint); + self._handler(client.getOptions().stackParser, event, hint); } + + // 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) { + await contextLines.addSourceContext(event); + } + return event; }); } @@ -57,53 +64,31 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - private _handler(stackParser: StackParser, event: Event, hint: EventHint): PromiseLike { - if (!event.exception || !event.exception.values || !isInstanceOf(hint.originalException, Error)) { - return resolvedSyncPromise(event); + private _handler(stackParser: StackParser, event: Event, hint: EventHint): Event { + if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) { + return event; } - return new SyncPromise(resolve => { - void this._walkErrorTree(stackParser, hint.originalException as Error, this._key) - .then((linkedErrors: Exception[]) => { - if (event && event.exception && event.exception.values) { - event.exception.values = [...linkedErrors, ...event.exception.values]; - } - resolve(event); - }) - .then(null, () => { - resolve(event); - }); - }); + const linkedErrors = this._walkErrorTree(stackParser, hint.originalException as ExtendedError, this._key); + event.exception.values = [...linkedErrors, ...event.exception.values]; + return event; } /** * @inheritDoc */ - private async _walkErrorTree( + private _walkErrorTree( stackParser: StackParser, error: ExtendedError, key: string, stack: Exception[] = [], - ): Promise { + ): Exception[] { if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) { - return Promise.resolve(stack); + return stack; } const exception = exceptionFromError(stackParser, error[key]); - // 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(stackParser, error[key], key, [exception, ...stack]) - .then(resolve) - .then(null, () => { - reject(); - }); - }); + return this._walkErrorTree(stackParser, error[key], key, [exception, ...stack]); } } diff --git a/packages/node/test/integrations/linkederrors.test.ts b/packages/node/test/integrations/linkederrors.test.ts index 6582c0f6e9e5..f59cf068e3de 100644 --- a/packages/node/test/integrations/linkederrors.test.ts +++ b/packages/node/test/integrations/linkederrors.test.ts @@ -20,10 +20,9 @@ describe('LinkedErrors', () => { const event = { message: 'foo', }; - return linkedErrors._handler(stackParser, event, {}).then((result: any) => { - expect(spy.mock.calls.length).toEqual(0); - expect(result).toEqual(event); - }); + const result = linkedErrors._handler(stackParser, event, {}); + expect(spy.mock.calls.length).toEqual(0); + expect(result).toEqual(event); }); it('should bail out if event contains exception, but no hint', async () => { @@ -47,24 +46,17 @@ describe('LinkedErrors', () => { it('should call walkErrorTree if event contains exception and hint with originalException', async () => { expect.assertions(1); - const spy = jest.spyOn(linkedErrors, '_walkErrorTree').mockImplementation( - async () => - new Promise<[]>(resolve => { - resolve([]); - }), - ); + const spy = jest.spyOn(linkedErrors, '_walkErrorTree').mockImplementation(() => []); const one = new Error('originalException'); const options = getDefaultNodeClientOptions({ stackParser }); const client = new NodeClient(options); - return client.eventFromException(one).then(event => - linkedErrors - ._handler(stackParser, event, { - originalException: one, - }) - .then((_: any) => { - expect(spy.mock.calls.length).toEqual(1); - }), - ); + return client.eventFromException(one).then(event => { + linkedErrors._handler(stackParser, event, { + originalException: one, + }); + + expect(spy.mock.calls.length).toEqual(1); + }); }); it('should recursively walk error to find linked exceptions and assign them to the event', async () => { @@ -77,24 +69,22 @@ describe('LinkedErrors', () => { const options = getDefaultNodeClientOptions({ stackParser }); const client = new NodeClient(options); - return client.eventFromException(one).then(event => - linkedErrors - ._handler(stackParser, event, { - originalException: one, - }) - .then((result: any) => { - expect(result.exception.values.length).toEqual(3); - expect(result.exception.values[0].type).toEqual('SyntaxError'); - expect(result.exception.values[0].value).toEqual('three'); - expect(result.exception.values[0].stacktrace).toHaveProperty('frames'); - expect(result.exception.values[1].type).toEqual('TypeError'); - expect(result.exception.values[1].value).toEqual('two'); - expect(result.exception.values[1].stacktrace).toHaveProperty('frames'); - expect(result.exception.values[2].type).toEqual('Error'); - expect(result.exception.values[2].value).toEqual('one'); - expect(result.exception.values[2].stacktrace).toHaveProperty('frames'); - }), - ); + return client.eventFromException(one).then(event => { + const result = linkedErrors._handler(stackParser, event, { + originalException: one, + }); + + expect(result.exception.values.length).toEqual(3); + expect(result.exception.values[0].type).toEqual('SyntaxError'); + expect(result.exception.values[0].value).toEqual('three'); + expect(result.exception.values[0].stacktrace).toHaveProperty('frames'); + expect(result.exception.values[1].type).toEqual('TypeError'); + expect(result.exception.values[1].value).toEqual('two'); + expect(result.exception.values[1].stacktrace).toHaveProperty('frames'); + expect(result.exception.values[2].type).toEqual('Error'); + expect(result.exception.values[2].value).toEqual('one'); + expect(result.exception.values[2].stacktrace).toHaveProperty('frames'); + }); }); it('should allow to change walk key', async () => { @@ -111,24 +101,22 @@ describe('LinkedErrors', () => { const options = getDefaultNodeClientOptions({ stackParser }); const client = new NodeClient(options); - return client.eventFromException(one).then(event => - linkedErrors - ._handler(stackParser, event, { - originalException: one, - }) - .then((result: any) => { - expect(result.exception.values.length).toEqual(3); - expect(result.exception.values[0].type).toEqual('SyntaxError'); - expect(result.exception.values[0].value).toEqual('three'); - expect(result.exception.values[0].stacktrace).toHaveProperty('frames'); - expect(result.exception.values[1].type).toEqual('TypeError'); - expect(result.exception.values[1].value).toEqual('two'); - expect(result.exception.values[1].stacktrace).toHaveProperty('frames'); - expect(result.exception.values[2].type).toEqual('Error'); - expect(result.exception.values[2].value).toEqual('one'); - expect(result.exception.values[2].stacktrace).toHaveProperty('frames'); - }), - ); + return client.eventFromException(one).then(event => { + const result = linkedErrors._handler(stackParser, event, { + originalException: one, + }); + + expect(result.exception.values.length).toEqual(3); + expect(result.exception.values[0].type).toEqual('SyntaxError'); + expect(result.exception.values[0].value).toEqual('three'); + expect(result.exception.values[0].stacktrace).toHaveProperty('frames'); + expect(result.exception.values[1].type).toEqual('TypeError'); + expect(result.exception.values[1].value).toEqual('two'); + expect(result.exception.values[1].stacktrace).toHaveProperty('frames'); + expect(result.exception.values[2].type).toEqual('Error'); + expect(result.exception.values[2].value).toEqual('one'); + expect(result.exception.values[2].stacktrace).toHaveProperty('frames'); + }); }); it('should allow to change stack size limit', async () => { @@ -145,21 +133,19 @@ describe('LinkedErrors', () => { const options = getDefaultNodeClientOptions({ stackParser }); const client = new NodeClient(options); - return client.eventFromException(one).then(event => - linkedErrors - ._handler(stackParser, event, { - originalException: one, - }) - .then((result: any) => { - expect(result.exception.values.length).toEqual(2); - expect(result.exception.values[0].type).toEqual('TypeError'); - expect(result.exception.values[0].value).toEqual('two'); - expect(result.exception.values[0].stacktrace).toHaveProperty('frames'); - expect(result.exception.values[1].type).toEqual('Error'); - expect(result.exception.values[1].value).toEqual('one'); - expect(result.exception.values[1].stacktrace).toHaveProperty('frames'); - }), - ); + return client.eventFromException(one).then(event => { + const result = linkedErrors._handler(stackParser, event, { + originalException: one, + }); + + expect(result.exception.values.length).toEqual(2); + expect(result.exception.values[0].type).toEqual('TypeError'); + expect(result.exception.values[0].value).toEqual('two'); + expect(result.exception.values[0].stacktrace).toHaveProperty('frames'); + expect(result.exception.values[1].type).toEqual('Error'); + expect(result.exception.values[1].value).toEqual('one'); + expect(result.exception.values[1].stacktrace).toHaveProperty('frames'); + }); }); }); });