Skip to content

Commit 418ae83

Browse files
kamilogorekkamilogorek
authored
[fix] Unify browser and node linked errors, correct order, fix limit handling (#1704)
1 parent c69749d commit 418ae83

File tree

3 files changed

+67
-61
lines changed

3 files changed

+67
-61
lines changed

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ describe('LinkedErrors', () => {
102102
originalException,
103103
});
104104

105-
// It shouldn't include root exception, as it's already processed in the event by the main error handler
106105
expect(result!.exception!.values!.length).equal(3);
107106
expect(result!.exception!.values![0].type).equal('SyntaxError');
108107
expect(result!.exception!.values![0].value).equal('three');
@@ -120,28 +119,24 @@ describe('LinkedErrors', () => {
120119
limit: 2,
121120
});
122121

123-
const three: ExtendedError = new SyntaxError('three');
124-
125-
const two: ExtendedError = new TypeError('two');
126-
two.cause = three;
127-
128122
const one: ExtendedError = new Error('one');
123+
const two: ExtendedError = new TypeError('two');
124+
const three: ExtendedError = new SyntaxError('three');
129125
one.cause = two;
126+
two.cause = three;
130127

131-
const originalException = one;
132128
const backend = new BrowserBackend({});
133-
const event = await backend.eventFromException(originalException);
129+
const event = await backend.eventFromException(one);
134130
const result = linkedErrors.handler(event, {
135-
originalException,
131+
originalException: one,
136132
});
137133

138-
// It shouldn't include root exception, as it's already processed in the event by the main error handler
139134
expect(result!.exception!.values!.length).equal(2);
140-
expect(result!.exception!.values![0].type).equal('Error');
141-
expect(result!.exception!.values![0].value).equal('one');
135+
expect(result!.exception!.values![0].type).equal('TypeError');
136+
expect(result!.exception!.values![0].value).equal('two');
142137
expect(result!.exception!.values![0].stacktrace).to.have.property('frames');
143-
expect(result!.exception!.values![1].type).equal('TypeError');
144-
expect(result!.exception!.values![1].value).equal('two');
138+
expect(result!.exception!.values![1].type).equal('Error');
139+
expect(result!.exception!.values![1].value).equal('one');
145140
expect(result!.exception!.values![1].stacktrace).to.have.property('frames');
146141
});
147142
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export class LinkedErrors implements Integration {
6262
return event;
6363
}
6464
const linkedErrors = await this.walkErrorTree(hint.originalException, this.key);
65-
event.exception.values = [...event.exception.values, ...linkedErrors];
65+
event.exception.values = [...linkedErrors, ...event.exception.values];
6666
return event;
6767
}
6868

@@ -74,10 +74,10 @@ export class LinkedErrors implements Integration {
7474
key: string,
7575
stack: SentryException[] = [],
7676
): Promise<SentryException[]> {
77-
if (!(error[key] instanceof Error) || stack.length >= this.limit) {
77+
if (!(error[key] instanceof Error) || stack.length + 1 >= this.limit) {
7878
return stack;
7979
}
8080
const exception = await getExceptionFromError(error[key]);
81-
return this.walkErrorTree(error[key], key, [...stack, exception]);
81+
return this.walkErrorTree(error[key], key, [exception, ...stack]);
8282
}
8383
}

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

Lines changed: 55 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { NodeBackend } from '../../src';
12
import { LinkedErrors } from '../../src/integrations/linkederrors';
23

34
let linkedErrors: LinkedErrors;
@@ -24,12 +25,9 @@ describe('LinkedErrors', () => {
2425

2526
it('should bail out if event contains exception, but no hint', async () => {
2627
const spy = jest.spyOn(linkedErrors, 'walkErrorTree');
27-
const event = {
28-
exception: {
29-
values: [],
30-
},
31-
message: 'foo',
32-
};
28+
const one = new Error('originalException');
29+
const backend = new NodeBackend({});
30+
const event = await backend.eventFromException(one);
3331
const result = await linkedErrors.handler(event);
3432
expect(spy.mock.calls.length).toEqual(0);
3533
expect(result).toEqual(event);
@@ -42,79 +40,92 @@ describe('LinkedErrors', () => {
4240
resolve([]);
4341
}),
4442
);
45-
const event = {
46-
exception: {
47-
values: [],
48-
},
49-
message: 'foo',
50-
};
51-
const hint = {
52-
originalException: new Error('originalException'),
53-
};
54-
await linkedErrors.handler(event, hint);
43+
const one = new Error('originalException');
44+
const backend = new NodeBackend({});
45+
const event = await backend.eventFromException(one);
46+
await linkedErrors.handler(event, {
47+
originalException: one,
48+
});
5549
expect(spy.mock.calls.length).toEqual(1);
5650
});
5751

5852
it('should recursively walk error to find linked exceptions and assign them to the event', async () => {
59-
const event = {
60-
exception: {
61-
values: [],
62-
},
63-
message: 'foo',
64-
};
65-
6653
const one: ExtendedError = new Error('one');
6754
const two: ExtendedError = new TypeError('two');
6855
const three: ExtendedError = new SyntaxError('three');
69-
70-
const originalException = one;
7156
one.cause = two;
7257
two.cause = three;
7358

59+
const backend = new NodeBackend({});
60+
const event = await backend.eventFromException(one);
7461
const result = await linkedErrors.handler(event, {
75-
originalException,
62+
originalException: one,
7663
});
7764

78-
// It shouldn't include root exception, as it's already processed in the event by the main error handler
79-
expect(result!.exception!.values!.length).toEqual(2);
80-
expect(result!.exception!.values![0].type).toEqual('TypeError');
81-
expect(result!.exception!.values![0].value).toEqual('two');
65+
expect(result!.exception!.values!.length).toEqual(3);
66+
expect(result!.exception!.values![0].type).toEqual('SyntaxError');
67+
expect(result!.exception!.values![0].value).toEqual('three');
8268
expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames');
83-
expect(result!.exception!.values![1].type).toEqual('SyntaxError');
84-
expect(result!.exception!.values![1].value).toEqual('three');
69+
expect(result!.exception!.values![1].type).toEqual('TypeError');
70+
expect(result!.exception!.values![1].value).toEqual('two');
8571
expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames');
72+
expect(result!.exception!.values![2].type).toEqual('Error');
73+
expect(result!.exception!.values![2].value).toEqual('one');
74+
expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames');
8675
});
8776

8877
it('should allow to change walk key', async () => {
8978
linkedErrors = new LinkedErrors({
9079
key: 'reason',
9180
});
92-
const event = {
93-
exception: {
94-
values: [],
95-
},
96-
message: 'foo',
97-
};
9881

9982
const one: ExtendedError = new Error('one');
10083
const two: ExtendedError = new TypeError('two');
10184
const three: ExtendedError = new SyntaxError('three');
102-
103-
const originalException = one;
10485
one.reason = two;
10586
two.reason = three;
10687

88+
const backend = new NodeBackend({});
89+
const event = await backend.eventFromException(one);
90+
const result = await linkedErrors.handler(event, {
91+
originalException: one,
92+
});
93+
94+
expect(result!.exception!.values!.length).toEqual(3);
95+
expect(result!.exception!.values![0].type).toEqual('SyntaxError');
96+
expect(result!.exception!.values![0].value).toEqual('three');
97+
expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames');
98+
expect(result!.exception!.values![1].type).toEqual('TypeError');
99+
expect(result!.exception!.values![1].value).toEqual('two');
100+
expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames');
101+
expect(result!.exception!.values![2].type).toEqual('Error');
102+
expect(result!.exception!.values![2].value).toEqual('one');
103+
expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames');
104+
});
105+
106+
it('should allow to change stack size limit', async () => {
107+
linkedErrors = new LinkedErrors({
108+
limit: 2,
109+
});
110+
111+
const one: ExtendedError = new Error('one');
112+
const two: ExtendedError = new TypeError('two');
113+
const three: ExtendedError = new SyntaxError('three');
114+
one.cause = two;
115+
two.cause = three;
116+
117+
const backend = new NodeBackend({});
118+
const event = await backend.eventFromException(one);
107119
const result = await linkedErrors.handler(event, {
108-
originalException,
120+
originalException: one,
109121
});
110122

111-
// It shouldn't include root exception, as it's already processed in the event by the main error handler
112123
expect(result!.exception!.values!.length).toEqual(2);
113124
expect(result!.exception!.values![0].type).toEqual('TypeError');
114125
expect(result!.exception!.values![0].value).toEqual('two');
115126
expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames');
116-
expect(result!.exception!.values![1].type).toEqual('SyntaxError');
117-
expect(result!.exception!.values![1].value).toEqual('three');
127+
expect(result!.exception!.values![1].type).toEqual('Error');
128+
expect(result!.exception!.values![1].value).toEqual('one');
118129
expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames');
119130
});
120131
});

0 commit comments

Comments
 (0)