Skip to content

Commit bbd81b6

Browse files
kamilogorekkamilogorek
authored
fix: Make RewriteFrame integration process all exceptions (#2362)
1 parent ef7b3b2 commit bbd81b6

File tree

3 files changed

+154
-43
lines changed

3 files changed

+154
-43
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
Coming soon...
66

7+
- [integrations] fix: Make RewriteFrame integration process all exceptions (#2362)
8+
79
## 5.10.2
810

911
- [browser] fix: Always trigger default browser onerror handler (#2348)

‎packages/integrations/src/rewriteframes.ts‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
1+
import { Event, EventProcessor, Hub, Integration, StackFrame, Stacktrace } from '@sentry/types';
22
import { basename, relative } from '@sentry/utils';
33

44
type StackFrameIteratee = (frame: StackFrame) => StackFrame;
@@ -69,29 +69,54 @@ export class RewriteFrames implements Integration {
6969

7070
/** JSDoc */
7171
public process(event: Event): Event {
72-
const frames = this._getFramesFromEvent(event);
73-
if (frames) {
74-
for (const i in frames) {
75-
// tslint:disable-next-line
76-
frames[i] = this._iteratee(frames[i]);
77-
}
72+
if (event.exception && Array.isArray(event.exception.values)) {
73+
return this._processExceptionsEvent(event);
74+
}
75+
76+
if (event.stacktrace) {
77+
return this._processStacktraceEvent(event);
7878
}
79+
7980
return event;
8081
}
8182

8283
/** JSDoc */
83-
private _getFramesFromEvent(event: Event): StackFrame[] | undefined {
84-
const exception = event.exception;
84+
private _processExceptionsEvent(event: Event): Event {
85+
try {
86+
return {
87+
...event,
88+
exception: {
89+
...event.exception,
90+
// The check for this is performed inside `process` call itself, safe to skip here
91+
// tslint:disable-next-line:no-non-null-assertion
92+
values: event.exception!.values!.map(value => ({
93+
...value,
94+
stacktrace: this._processStacktrace(value.stacktrace),
95+
})),
96+
},
97+
};
98+
} catch (_oO) {
99+
return event;
100+
}
101+
}
85102

86-
if (exception) {
87-
try {
88-
return exception.values && exception.values[0].stacktrace && exception.values[0].stacktrace.frames;
89-
} catch (_oO) {
90-
return undefined;
91-
}
92-
} else if (event.stacktrace) {
93-
return event.stacktrace.frames;
103+
/** JSDoc */
104+
private _processStacktraceEvent(event: Event): Event {
105+
try {
106+
return {
107+
...event,
108+
stacktrace: this._processStacktrace(event.stacktrace),
109+
};
110+
} catch (_oO) {
111+
return event;
94112
}
95-
return undefined;
113+
}
114+
115+
/** JSDoc */
116+
private _processStacktrace(stacktrace?: Stacktrace): Stacktrace {
117+
return {
118+
...stacktrace,
119+
frames: stacktrace && stacktrace.frames && stacktrace.frames.map(f => this._iteratee(f)),
120+
};
96121
}
97122
}

‎packages/integrations/test/rewriteframes.test.ts‎

Lines changed: 109 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,52 +6,53 @@ let rewriteFrames: RewriteFrames;
66
let messageEvent: Event;
77
let exceptionEvent: Event;
88
let windowsExceptionEvent: Event;
9+
let multipleStacktracesEvent: Event;
910

1011
describe('RewriteFrames', () => {
1112
beforeEach(() => {
1213
messageEvent = {
1314
stacktrace: {
14-
frames: [
15-
{
16-
filename: '/www/src/app/file1.js',
17-
},
15+
frames: [{ filename: '/www/src/app/file1.js' }, { filename: '/www/src/app/mo\\dule/file2.js' }],
16+
},
17+
};
18+
exceptionEvent = {
19+
exception: {
20+
values: [
1821
{
19-
filename: '/www/src/app/mo\\dule/file2.js',
22+
stacktrace: {
23+
frames: [{ filename: '/www/src/app/file1.js' }, { filename: '/www/src/app/mo\\dule/file2.js' }],
24+
},
2025
},
2126
],
2227
},
2328
};
24-
exceptionEvent = {
29+
windowsExceptionEvent = {
2530
exception: {
2631
values: [
2732
{
2833
stacktrace: {
29-
frames: [
30-
{
31-
filename: '/www/src/app/file1.js',
32-
},
33-
{
34-
filename: '/www/src/app/mo\\dule/file2.js',
35-
},
36-
],
34+
frames: [{ filename: 'C:\\www\\src\\app\\file1.js' }, { filename: 'C:\\www\\src\\app\\file2.js' }],
3735
},
3836
},
3937
],
4038
},
4139
};
42-
windowsExceptionEvent = {
40+
multipleStacktracesEvent = {
4341
exception: {
4442
values: [
4543
{
4644
stacktrace: {
47-
frames: [
48-
{
49-
filename: 'C:\\www\\src\\app\\file1.js',
50-
},
51-
{
52-
filename: 'C:\\www\\src\\app\\file2.js',
53-
},
54-
],
45+
frames: [{ filename: '/www/src/app/file1.js' }, { filename: '/www/src/app/mo\\dule/file2.js' }],
46+
},
47+
},
48+
{
49+
stacktrace: {
50+
frames: [{ filename: '/www/src/app/file3.js' }, { filename: '/www/src/app/mo\\dule/file4.js' }],
51+
},
52+
},
53+
{
54+
stacktrace: {
55+
frames: [{ filename: '/www/src/app/file5.js' }, { filename: '/www/src/app/mo\\dule/file6.js' }],
5556
},
5657
},
5758
],
@@ -125,20 +126,103 @@ describe('RewriteFrames', () => {
125126
});
126127
});
127128

128-
it('transforms messageEvent frames', async () => {
129+
it('transforms messageEvent frames', () => {
129130
const event = rewriteFrames.process(messageEvent);
130131
expect(event.stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
131132
expect(event.stacktrace!.frames![0].function).toEqual('whoops');
132133
expect(event.stacktrace!.frames![1].filename).toEqual('/www/src/app/mo\\dule/file2.js');
133134
expect(event.stacktrace!.frames![1].function).toEqual('whoops');
134135
});
135136

136-
it('transforms exceptionEvent frames', async () => {
137+
it('transforms exceptionEvent frames', () => {
137138
const event = rewriteFrames.process(exceptionEvent);
138139
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
139140
expect(event.exception!.values![0].stacktrace!.frames![0].function).toEqual('whoops');
140141
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/mo\\dule/file2.js');
141142
expect(event.exception!.values![0].stacktrace!.frames![1].function).toEqual('whoops');
142143
});
143144
});
145+
146+
describe('can process events that contain multiple stacktraces', () => {
147+
it('with defaults', () => {
148+
rewriteFrames = new RewriteFrames();
149+
const event = rewriteFrames.process(multipleStacktracesEvent);
150+
// first stacktrace
151+
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js');
152+
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js');
153+
// second stacktrace
154+
expect(event.exception!.values![1].stacktrace!.frames![0].filename).toEqual('app:///file3.js');
155+
expect(event.exception!.values![1].stacktrace!.frames![1].filename).toEqual('app:///file4.js');
156+
// third stacktrace
157+
expect(event.exception!.values![2].stacktrace!.frames![0].filename).toEqual('app:///file5.js');
158+
expect(event.exception!.values![2].stacktrace!.frames![1].filename).toEqual('app:///file6.js');
159+
});
160+
161+
it('with custom root', () => {
162+
rewriteFrames = new RewriteFrames({
163+
root: '/www',
164+
});
165+
const event = rewriteFrames.process(multipleStacktracesEvent);
166+
// first stacktrace
167+
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
168+
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/mo\\dule/file2.js');
169+
// second stacktrace
170+
expect(event.exception!.values![1].stacktrace!.frames![0].filename).toEqual('app:///src/app/file3.js');
171+
expect(event.exception!.values![1].stacktrace!.frames![1].filename).toEqual('app:///src/app/mo\\dule/file4.js');
172+
// third stacktrace
173+
expect(event.exception!.values![2].stacktrace!.frames![0].filename).toEqual('app:///src/app/file5.js');
174+
expect(event.exception!.values![2].stacktrace!.frames![1].filename).toEqual('app:///src/app/mo\\dule/file6.js');
175+
});
176+
177+
it('with custom iteratee', () => {
178+
rewriteFrames = new RewriteFrames({
179+
iteratee: (frame: StackFrame) => ({
180+
...frame,
181+
function: 'whoops',
182+
}),
183+
});
184+
const event = rewriteFrames.process(multipleStacktracesEvent);
185+
// first stacktrace
186+
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
187+
expect(event.exception!.values![0].stacktrace!.frames![0].function).toEqual('whoops');
188+
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/mo\\dule/file2.js');
189+
expect(event.exception!.values![0].stacktrace!.frames![1].function).toEqual('whoops');
190+
// second stacktrace
191+
expect(event.exception!.values![1].stacktrace!.frames![0].filename).toEqual('/www/src/app/file3.js');
192+
expect(event.exception!.values![1].stacktrace!.frames![0].function).toEqual('whoops');
193+
expect(event.exception!.values![1].stacktrace!.frames![1].filename).toEqual('/www/src/app/mo\\dule/file4.js');
194+
expect(event.exception!.values![1].stacktrace!.frames![1].function).toEqual('whoops');
195+
// third stacktrace
196+
expect(event.exception!.values![2].stacktrace!.frames![0].filename).toEqual('/www/src/app/file5.js');
197+
expect(event.exception!.values![2].stacktrace!.frames![0].function).toEqual('whoops');
198+
expect(event.exception!.values![2].stacktrace!.frames![1].filename).toEqual('/www/src/app/mo\\dule/file6.js');
199+
expect(event.exception!.values![2].stacktrace!.frames![1].function).toEqual('whoops');
200+
});
201+
});
202+
203+
describe('bails when unable to extract frames', () => {
204+
it('no exception values', () => {
205+
rewriteFrames = new RewriteFrames({});
206+
const brokenEvent = {
207+
exception: {
208+
values: undefined,
209+
},
210+
};
211+
expect(rewriteFrames.process(brokenEvent)).toEqual(brokenEvent);
212+
});
213+
214+
it('no frames', () => {
215+
rewriteFrames = new RewriteFrames({});
216+
const brokenEvent = {
217+
exception: {
218+
values: [
219+
{
220+
stacktrace: {},
221+
},
222+
],
223+
},
224+
};
225+
expect(rewriteFrames.process(brokenEvent)).toEqual(brokenEvent);
226+
});
227+
});
144228
});

0 commit comments

Comments
 (0)