Skip to content

Commit

Permalink
fix(node): Apply source context to linked errors even when it is unca…
Browse files Browse the repository at this point in the history 
…ched (#8453)
  • Loading branch information
@lforst
lforst committed Jul 5, 2023
1 parent 4b98349 commit c55943f
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 106 deletions.
57 changes: 21 additions & 36 deletions packages/node/src/integrations/linkederrors.ts
View file
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -46,64 +45,50 @@ export class LinkedErrors implements Integration {
addGlobalEventProcessor(async (event: Event, hint: EventHint) => {
const hub = getCurrentHub();
const self = hub.getIntegration(LinkedErrors);
const client = hub.getClient<NodeClient>();
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;
});
}

/**
* @inheritDoc
*/
private _handler(stackParser: StackParser, event: Event, hint: EventHint): PromiseLike<Event> {
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<Event>(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[]> {
): 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<Exception[]>((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]);
}
}
126 changes: 56 additions & 70 deletions packages/node/test/integrations/linkederrors.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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');
});
});
});
});

0 comments on commit c55943f

Please sign in to comment.