8000 fix: Add type/value to exception in case of no error (#1803) · productinfo/sentry-javascript@44ce010 · GitHub
[go: up one dir, main page]
Skip to content

Commit 44ce010

Browse files
HazATHazAT
authored
fix: Add type/value to exception in case of no error (getsentry#1803)
* fix: Add type/value to exception in case of no error * fix: Init first element in array * fix: Test and stacktrace * fix: Unhandled promise rejections
1 parent 73b8d65 commit 44ce010

File tree

8 files changed

+59
-6
lines changed

8 files changed

+59
-6
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
`7` lines pre/post
1111
- [core]: ref: Change way how transports are initialized
1212
- [core]: ref: Rename `RequestBuffer` to `PromiseBuffer`, also introduce limit
13+
- [browser] fix: Prevent empty exception values
1314

1415
## 4.4.2
1516

‎packages/browser/src/backend.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { BaseBackend, Options, SentryError } from '@sentry/core';
22
import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types';
33
import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is';
44
import { supportsBeacon, supportsFetch } from '@sentry/utils/supports';
5-
import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
5+
import { addExceptionTypeValue, eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
66
import { computeStackTrace } from './tracekit';
77
import { BeaconTransport, FetchTransport, XHRTransport } from './transports';
88

@@ -87,6 +87,7 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
8787
const message = ex.message ? `${name}: ${ex.message}` : name;
8888

8989
event = await this.eventFromMessage(message, undefined, hint);
90+
addExceptionTypeValue(event, message);
9091
} else if (isError(exception as Error)) {
9192
// we have a real Error object, do nothing
9293
event = eventFromStacktrace(computeStackTrace(exception as Error));
@@ -96,6 +97,7 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
9697
// which is much better than creating new group when any key/value change
9798
const ex = exception as {};
9899
event = eventFromPlainObject(ex, hint.syntheticException);
100+
addExceptionTypeValue(event, 'Custom Object');
99101
} else {
100102
// If none of previous checks were valid, then it means that
101103
// it's not a DOMError/DOMException
@@ -105,6 +107,7 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
105107
// So bail out and capture it as a simple message:
106108
const ex = exception as string;
107109
event = await this.eventFromMessage(ex, undefined, hint);
110+
addExceptionTypeValue(event, `${< 8000 span class=pl-s1>ex}`);
108111
}
109112

110113
event = {

‎packages/browser/src/parsers.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,16 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
9292
.slice(0, STACKTRACE_LIMIT)
9393
.reverse();
9494
}
95+
96+
/**
97+
* Adds exception values, type and value to an synthetic Exception.
98+
* @param event The event to modify.
99+
* @param message Message to be added.
100+
*/
101+
export function addExceptionTypeValue(event: SentryEvent, message: string): void {
102+
event.exception = event.exception || {};
103+
event.exception.values = event.exception.values || [];
104+
event.exception.values[0] = event.exception.values[0] || {};
105+
event.exception.values[0].value = event.exception.values[0].value || message;
106+
event.exception.values[0].type = event.exception.values[0].type || 'Error';
107+
}

‎packages/browser/test/integration/test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,25 @@ for (var idx in frames) {
244244
);
245245
});
246246

247+
it('should have exception with type and value', function(done) {
248+
var iframe = this.iframe;
249+
iframeExecute(
250+
iframe,
251+
done,
252+
function() {
253+
Sentry.captureException('this is my test exception');
254+
},
255+
function(sentryData) {
256+
if (debounceAssertEventCount(sentryData, 1, done)) {
257+
var sentryData = sentryData[0];
258+
assert.isNotEmpty(sentryData.exception.values[0].value);
259+
assert.isNotEmpty(sentryData.exception.values[0].type);
260+
done();
261+
}
262+
}
263+
);
264+
});
265+
247266
it('should reject duplicate, back-to-back errors from captureException', function(done) {
248267
var iframe = this.iframe;
249268
iframeExecute(

‎packages/core/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
"fix:prettier": "prettier --write \"{src,test}/**/*.ts\"",
4343
"fix:tslint": "tslint --fix -t stylish -p .",
4444
"test": "jest",
45-
"test:watch": "jest --watch --notify"
45+
"test:watch": "jest --watch"
4646
},
4747
"jest": {
4848
"collectCoverage": true,

‎packages/core/src/promisebuffer.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,14 @@ export class PromiseBuffer<T> {
2727
if (this.buffer.indexOf(task) === -1) {
2828
this.buffer.push(task);
2929
}
30-
task.then(async () => this.remove(task)).catch(async () => this.remove(task));
30+
task
31+
.then(async () => this.remove(task))
32+
.catch(async () =>
33+
this.remove(task).catch(() => {
34+
// We have to add this catch here otherwise we have an unhandledPromiseRejection
35+
// because it's a new Promise chain.
36+
}),
37+
);
3138
return task;
3239
}
3340

‎packages/core/test/lib/promisebuffer.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,14 @@ describe('PromiseBuffer', () => {
9292
});
9393
jest.runAllTimers();
9494
});
95+
96+
test('rejecting', async () => {
97+
expect.assertions(1);
98+
const q = new PromiseBuffer<void>();
99+
const p = new Promise<void>((_, reject) => setTimeout(reject, 1));
100+
jest.runAllTimers();
101+
return q.add(p).catch(() => {
102+
expect(true).toBe(true);
103+
}) FD56 ;
104+
});
95105
});

‎packages/node/test/index.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ describe('SentryNode', () => {
127127
// for our own captureMessage/captureException calls yet
128128
expect(event.breadcrumbs!).toHaveLength(2);
129129
done();
130-
return event;
130+
return null;
131131
},
132132
dsn,
133133
});
@@ -164,7 +164,7 @@ describe('SentryNode', () => {
164164
expect(event.exception!.values![0].value).toBe('test');
165165
expect(event.exception!.values![0].stacktrace).toBeTruthy();
166166
done();
167-
return event;
167+
return null;
168168
},
169169
dsn,
170170
}),
@@ -249,7 +249,7 @@ describe('SentryNode', () => {
249249
expect(event.message).toBe('test domain');
250250
expect(event.exception).toBeUndefined();
251251
done();
252-
return event;
252+
return null;
253253
},
254254
dsn,
255255
});

0 commit comments

Comments
 (0)
0