Skip to content

Commit c55943f

Browse files
author
Luca Forstner
authored
fix(node): Apply source context to linked errors even when it is uncached (#8453)
1 parent 4b98349 commit c55943f

File tree

2 files changed

+77
-106
lines changed

2 files changed

+77
-106
lines changed

‎packages/node/src/integrations/linkederrors.ts

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
22
import type { Event, EventHint, Exception, ExtendedError, Integration, StackParser } from '@sentry/types';
3-
import { isInstanceOf, resolvedSyncPromise, SyncPromise } from '@sentry/utils';
3+
import { isInstanceOf } from '@sentry/utils';
44

5-
import type { NodeClient } from '../client';
65
import { exceptionFromError } from '../eventbuilder';
76
import { ContextLines } from './contextlines';
87

@@ -46,64 +45,50 @@ export class LinkedErrors implements Integration {
4645
addGlobalEventProcessor(async (event: Event, hint: EventHint) => {
4746
const hub = getCurrentHub();
4847
const self = hub.getIntegration(LinkedErrors);
49-
const client = hub.getClient<NodeClient>();
48+
const client = hub.getClient();
5049
if (client && self && self._handler && typeof self._handler === 'function') {
51-
await self._handler(client.getOptions().stackParser, event, hint);
50+
self._handler(client.getOptions().stackParser, event, hint);
5251
}
52+
53+
// If the ContextLines integration is enabled, we add source code context to linked errors
54+
// because we can't guarantee the order that integrations are run.
55+
const contextLines = getCurrentHub().getIntegration(ContextLines);
56+
if (contextLines) {
57+
await contextLines.addSourceContext(event);
58+
}
59+
5360
return event;
5461
});
5562
}
5663

5764
/**
5865
* @inheritDoc
5966
*/
60-
private _handler(stackParser: StackParser, event: Event, hint: EventHint): PromiseLike<Event> {
61-
if (!event.exception || !event.exception.values || !isInstanceOf(hint.originalException, Error)) {
62-
return resolvedSyncPromise(event);
67+
private _handler(stackParser: StackParser, event: Event, hint: EventHint): Event {
68+
if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) {
69+
return event;
6370
}
6471

65-
return new SyncPromise<Event>(resolve => {
66-
void this._walkErrorTree(stackParser, hint.originalException as Error, this._key)
67-
.then((linkedErrors: Exception[]) => {
68-
if (event && event.exception && event.exception.values) {
69-
event.exception.values = [...linkedErrors, ...event.exception.values];
70-
}
71-
resolve(event);
72-
})
73-
.then(null, () => {
74-
resolve(event);
75-
});
76-
});
72+
const linkedErrors = this._walkErrorTree(stackParser, hint.originalException as ExtendedError, this._key);
73+
event.exception.values = [...linkedErrors, ...event.exception.values];
74+
return event;
7775
}
7876

7977
/**
8078
* @inheritDoc
8179
*/
82-
private async _walkErrorTree(
80+
private _walkErrorTree(
8381
stackParser: StackParser,
8482
error: ExtendedError,
8583
key: string,
8684
stack: Exception[] = [],
87-
): Promise<Exception[]> {
85+
): Exception[] {
8886
if (!isInstanceOf(error[key], Error) || stack.length + 1 >= this._limit) {
89-
return Promise.resolve(stack);
87+
return stack;
9088
}
9189

9290
const exception = exceptionFromError(stackParser, error[key]);
9391

94-
// If the ContextLines integration is enabled, we add source code context to linked errors
95-
// because we can't guarantee the order that integrations are run.
96-
const contextLines = getCurrentHub().getIntegration(ContextLines);
97-
if (contextLines && exception.stacktrace?.frames) {
98-
await contextLines.addSourceContextToFrames(exception.stacktrace.frames);
99-
}
100-
101-
return new Promise<Exception[]>((resolve, reject) => {
102-
void this._walkErrorTree(stackParser, error[key], key, [exception, ...stack])
103-
.then(resolve)
104-
.then(null, () => {
105-
reject();
106-
});
107-
});
92+
return this._walkErrorTree(stackParser, error[key], key, [exception, ...stack]);
10893
}
10994
}

‎packages/node/test/integrations/linkederrors.test.ts

Lines changed: 56 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@ describe('LinkedErrors', () => {
2020
const event = {
2121
message: 'foo',
2222
};
23-
return linkedErrors._handler(stackParser, event, {}).then((result: any) => {
24-
expect(spy.mock.calls.length).toEqual(0);
25-
expect(result).toEqual(event);
26-
});
23+
const result = linkedErrors._handler(stackParser, event, {});
24+
expect(spy.mock.calls.length).toEqual(0);
25+
expect(result).toEqual(event);
2726
});
2827

2928
it('should bail out if event contains exception, but no hint', async () => {
@@ -47,24 +46,17 @@ describe('LinkedErrors', () => {
4746

4847
it('should call walkErrorTree if event contains exception and hint with originalException', async () => {
4948
expect.assertions(1);
50-
const spy = jest.spyOn(linkedErrors, '_walkErrorTree').mockImplementation(
51-
async () =>
52-
new Promise<[]>(resolve => {
53-
resolve([]);
54-
}),
55-
);
49+
const spy = jest.spyOn(linkedErrors, '_walkErrorTree').mockImplementation(() => []);
5650
const one = new Error('originalException');
5751
const options = getDefaultNodeClientOptions({ stackParser });
5852
const client = new NodeClient(options);
59-
return client.eventFromException(one).then(event =>
60-
linkedErrors
61-
._handler(stackParser, event, {
62-
originalException: one,
63-
})
64-
.then((_: any) => {
65-
expect(spy.mock.calls.length).toEqual(1);
66-
}),
67-
);
53+
return client.eventFromException(one).then(event => {
54+
linkedErrors._handler(stackParser, event, {
55+
originalException: one,
56+
});
57+
58+
expect(spy.mock.calls.length).toEqual(1);
59+
});
6860
});
6961

7062
it('should recursively walk error to find linked exceptions and assign them to the event', async () => {
@@ -77,24 +69,22 @@ describe('LinkedErrors', () => {
7769

7870
const options = getDefaultNodeClientOptions({ stackParser });
7971
const client = new NodeClient(options);
80-
return client.eventFromException(one).then(event =>
81-
linkedErrors
82-
._handler(stackParser, event, {
83-
originalException: one,
84-
})
85-
.then((result: any) => {
86-
expect(result.exception.values.length).toEqual(3);
87-
expect(result.exception.values[0].type).toEqual('SyntaxError');
88-
expect(result.exception.values[0].value).toEqual('three');
89-
expect(result.exception.values[0].stacktrace).toHaveProperty('frames');
90-
expect(result.exception.values[1].type).toEqual('TypeError');
91-
expect(result.exception.values[1].value).toEqual('two');
92-
expect(result.exception.values[1].stacktrace).toHaveProperty('frames');
93-
expect(result.exception.values[2].type).toEqual('Error');
94-
expect(result.exception.values[2].value).toEqual('one');
95-
expect(result.exception.values[2].stacktrace).toHaveProperty('frames');
96-
}),
97-
);
72+
return client.eventFromException(one).then(event => {
73+
const result = linkedErrors._handler(stackParser, event, {
74+
originalException: one,
75+
});
76+
77+
expect(result.exception.values.length).toEqual(3);
78+
expect(result.exception.values[0].type).toEqual('SyntaxError');
79+
expect(result.exception.values[0].value).toEqual('three');
80+
expect(result.exception.values[0].stacktrace).toHaveProperty('frames');
81+
expect(result.exception.values[1].type).toEqual('TypeError');
82+
expect(result.exception.values[1].value).toEqual('two');
83+
expect(result.exception.values[1].stacktrace).toHaveProperty('frames');
84+
expect(result.exception.values[2].type).toEqual('Error');
85+
expect(result.exception.values[2].value).toEqual('one');
86+
expect(result.exception.values[2].stacktrace).toHaveProperty('frames');
87+
});
9888
});
9989

10090
it('should allow to change walk key', async () => {
@@ -111,24 +101,22 @@ describe('LinkedErrors', () => {
111101

112102
const options = getDefaultNodeClientOptions({ stackParser });
113103
const client = new NodeClient(options);
114-
return client.eventFromException(one).then(event =>
115-
linkedErrors
116-
._handler(stackParser, event, {
117-
originalException: one,
118-
})
119-
.then((result: any) => {
120-
expect(result.exception.values.length).toEqual(3);
121-
expect(result.exception.values[0].type).toEqual('SyntaxError');
122-
expect(result.exception.values[0].value).toEqual('three');
123-
expect(result.exception.values[0].stacktrace).toHaveProperty('frames');
124-
expect(result.exception.values[1].type).toEqual('TypeError');
125-
expect(result.exception.values[1].value).toEqual('two');
126-
expect(result.exception.values[1].stacktrace).toHaveProperty('frames');
127-
expect(result.exception.values[2].type).toEqual('Error');
128-
expect(result.exception.values[2].value).toEqual('one');
129-
expect(result.exception.values[2].stacktrace).toHaveProperty('frames');
130-
}),
131-
);
104+
return client.eventFromException(one).then(event => {
105+
const result = linkedErrors._handler(stackParser, event, {
106+
originalException: one,
107+
});
108+
109+
expect(result.exception.values.length).toEqual(3);
110+
expect(result.exception.values[0].type).toEqual('SyntaxError');
111+
expect(result.exception.values[0].value).toEqual('three');
112+
expect(result.exception.values[0].stacktrace).toHaveProperty('frames');
113+
expect(result.exception.values[1].type).toEqual('TypeError');
114+
expect(result.exception.values[1].value).toEqual('two');
115+
expect(result.exception.values[1].stacktrace).toHaveProperty('frames');
116+
expect(result.exception.values[2].type).toEqual('Error');
117+
expect(result.exception.values[2].value).toEqual('one');
118+
expect(result.exception.values[2].stacktrace).toHaveProperty('frames');
119+
});
132120
});
133121

134122
it('should allow to change stack size limit', async () => {
@@ -145,21 +133,19 @@ describe('LinkedErrors', () => {
145133

146134
const options = getDefaultNodeClientOptions({ stackParser });
147135
const client = new NodeClient(options);
148-
return client.eventFromException(one).then(event =>
149-
linkedErrors
150-
._handler(stackParser, event, {
151-
originalException: one,
152-
})
153-
.then((result: any) => {
154-
expect(result.exception.values.length).toEqual(2);
155-
expect(result.exception.values[0].type).toEqual('TypeError');
156-
expect(result.exception.values[0].value).toEqual('two');
157-
expect(result.exception.values[0].stacktrace).toHaveProperty('frames');
158-
expect(result.exception.values[1].type).toEqual('Error');
159-
expect(result.exception.values[1].value).toEqual('one');
160-
expect(result.exception.values[1].stacktrace).toHaveProperty('frames');
161-
}),
162-
);
136+
return client.eventFromException(one).then(event => {
137+
const result = linkedErrors._handler(stackParser, event, {
138+
originalException: one,
139+
});
140+
141+
expect(result.exception.values.length).toEqual(2);
142+
expect(result.exception.values[0].type).toEqual('TypeError');
143+
expect(result.exception.values[0].value).toEqual('two');
144+
expect(result.exception.values[0].stacktrace).toHaveProperty('frames');
145+
expect(result.exception.values[1].type).toEqual('Error');
146+
expect(result.exception.values[1].value).toEqual('one');
147+
expect(result.exception.values[1].stacktrace).toHaveProperty('frames');
148+
});
163149
});
164150
});
165151
});

0 commit comments

Comments
 (0)