Skip to content

Commit 2a3c409

Browse files
mydeamydea
committed
feat(core): Extract errors from props in unkown inputs
1 parent f6a3e02 commit 2a3c409

File tree

4 files changed

+283
-72
lines changed

4 files changed

+283
-72
lines changed

‎packages/browser/src/eventbuilder.ts‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,7 @@ export function exceptionFromError(stackParser: StackParser, ex: Error): Excepti
4848
return exception;
4949
}
5050

51-
/**
52-
* @hidden
53-
*/
54-
export function eventFromPlainObject(
51+
function eventFromPlainObject(
5552
stackParser: StackParser,
5653
exception: Record<string, unknown>,
5754
syntheticException?: Error,
@@ -60,35 +57,46 @@ export function eventFromPlainObject(
6057
const client = getClient();
6158
const normalizeDepth = client && client.getOptions().normalizeDepth;
6259

63-
const event: Event = {
60+
// If we can, we extract an exception from the object properties
61+
const errorFromProp = getErrorPropertyFromObject(exception);
62+
63+
const extra = {
64+
__serialized__: normalizeToSize(exception, normalizeDepth),
65+
};
66+
67+
if (errorFromProp) {
68+
return {
69+
exception: {
70+
values: [exceptionFromError(stackParser, errorFromProp)],
71+
},
72+
extra,
73+
};
74+
}
75+
76+
const event = {
6477
exception: {
6578
values: [
6679
{
6780
type: isEvent(exception) ? exception.constructor.name : isUnhandledRejection ? 'UnhandledRejection' : 'Error',
6881
value: getNonErrorObjectExceptionValue(exception, { isUnhandledRejection }),
69-
},
82+
} as Exception,
7083
],
7184
},
72-
extra: {
73-
__serialized__: normalizeToSize(exception, normalizeDepth),
74-
},
75-
};
85+
extra,
86+
} satisfies Event;
7687

7788
if (syntheticException) {
7889
const frames = parseStackFrames(stackParser, syntheticException);
7990
if (frames.length) {
8091
// event.exception.values[0] has been set above
81-
(event.exception as { values: Exception[] }).values[0].stacktrace = { frames };
92+
event.exception.values[0].stacktrace = { frames };
8293
}
8394
}
8495

8596
return event;
8697
}
8798

88-
/**
89-
* @hidden
90-
*/
91-
export function eventFromError(stackParser: StackParser, ex: Error): Event {
99+
function eventFromError(stackParser: StackParser, ex: Error): Event {
92100
return {
93101
exception: {
94102
values: [exceptionFromError(stackParser, ex)],
@@ -97,7 +105,7 @@ export function eventFromError(stackParser: StackParser, ex: Error): Event {
97105
}
98106

99107
/** Parses stack frames from an error */
100-
export function parseStackFrames(
108+
function parseStackFrames(
101109
stackParser: StackParser,
102110
ex: Error & { framesToPop?: number; stacktrace?: string },
103111
): StackFrame[] {
@@ -283,10 +291,7 @@ export function eventFromUnknownInput(
283291
return event;
284292
}
285293

286-
/**
287-
* @hidden
288-
*/
289-
export function eventFromString(
294+
function eventFromString(
290295
stackParser: StackParser,
291296
message: ParameterizedString,
292297
syntheticException?: Error,
@@ -346,3 +351,17 @@ function getObjectClassName(obj: unknown): string | undefined | void {
346351
// ignore errors here
347352
}
348353
}
354+
355+
/** If a plain object has a property that is an `Error`, return this error. */
356+
function getErrorPropertyFromObject(obj: Record<string, unknown>): Error | undefined {
357+
for (const prop in obj) {
358+
if (Object.prototype.hasOwnProperty.call(obj, prop)) {
359+
const value = obj[prop];
360+
if (value instanceof Error) {
361+
return value;
362+
}
363+
}
364+
}
365+
366+
return undefined;
367+
}

‎packages/browser/test/unit/eventbuilder.test.ts‎

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { defaultStackParser } from '../../src';
2-
import { eventFromPlainObject } from '../../src/eventbuilder';
2+
import { eventFromUnknownInput } from '../../src/eventbuilder';
33

44
jest.mock('@sentry/core', () => {
55
const original = jest.requireActual('@sentry/core');
@@ -12,17 +12,6 @@ jest.mock('@sentry/core', () => {
1212
},
1313
};
1414
},
15-
getCurrentHub() {
16-
return {
17-
getClient(): any {
18-
return {
19-
getOptions(): any {
20-
return { normalizeDepth: 6 };
21-
},
22-
};
23-
},
24-
};
25-
},
2615
};
2716
});
2817

@@ -35,7 +24,7 @@ afterEach(() => {
3524
jest.resetAllMocks();
3625
});
3726

38-
describe('eventFromPlainObject', () => {
27+
describe('eventFromUnknownInput', () => {
3928
it('should use normalizeDepth from init options', () => {
4029
const deepObject = {
4130
a: {
@@ -53,7 +42,7 @@ describe('eventFromPlainObject', () => {
5342
},
5443
};
5544

56-
const event = eventFromPlainObject(defaultStackParser, deepObject);
45+
const event = eventFromUnknownInput(defaultStackParser, deepObject);
5746

5847
expect(event?.extra?.__serialized__).toEqual({
5948
a: {
@@ -71,16 +60,78 @@ describe('eventFromPlainObject', () => {
7160
});
7261

7362
it.each([
74-
['empty object', {}, 'Object captured as exception with keys: [object has no keys]'],
75-
['pojo', { prop1: 'hello', prop2: 2 }, 'Object captured as exception with keys: prop1, prop2'],
76-
['Custom Class', new MyTestClass(), 'Object captured as exception with keys: prop1, prop2'],
77-
['Event', new Event('custom'), 'Event `Event` (type=custom) captured as exception'],
78-
['MouseEvent', new MouseEvent('click'), 'Event `MouseEvent` (type=click) captured as exception'],
79-
] as [string, Record<string, unknown>, string][])(
63+
['empty object', {}, {}, 'Object captured as exception with keys: [object has no keys]'],
64+
[
65+
'pojo',
66+
{ prop1: 'hello', prop2: 2 },
67+
{ prop1: 'hello', prop2: 2 },
68+
'Object captured as exception with keys: prop1, prop2',
69+
],
70+
[
71+
'Custom Class',
72+
new MyTestClass(),
73+
{ prop1: 'hello', prop2: 2 },
74+
'Object captured as exception with keys: prop1, prop2',
75+
],
76+
[
77+
'Event',
78+
new Event('custom'),
79+
{
80+
currentTarget: '[object Null]',
81+
isTrusted: false,
82+
target: '[object Null]',
83+
type: 'custom',
84+
},
85+
'Event `Event` (type=custom) captured as exception',
86+
],
87+
[
88+
'MouseEvent',
89+
new MouseEvent('click'),
90+
{
91+
currentTarget: '[object Null]',
92+
isTrusted: false,
93+
target: '[object Null]',
94+
type: 'click',
95+
},
96+
'Event `MouseEvent` (type=click) captured as exception',
97+
],
98+
] as [string, Record<string, unknown>, Record<string, unknown>, string][])(
8099
'has correct exception value for %s',
81-
(_name, exception, expected) => {
82-
const actual = eventFromPlainObject(defaultStackParser, exception);
100+
(_name, exception, serializedException, expected) => {
101+
const actual = eventFromUnknownInput(defaultStackParser, exception);
83102
expect(actual.exception?.values?.[0]?.value).toEqual(expected);
103+
104+
expect(actual.extra).toEqual({
105+
__serialized__: serializedException,
106+
});
84107
},
85108
);
109+
110+
it('handles object with error prop', () => {
111+
const error = new Error('Some error');
112+
const event = eventFromUnknownInput(defaultStackParser, {
113+
foo: { bar: 'baz' },
114+
name: 'BadType',
115+
err: error,
116+
});
117+
118+
expect(event.exception?.values?.[0]).toEqual(
119+
expect.objectContaining({
120+
mechanism: { handled: true, synthetic: true, type: 'generic' },
121+
type: 'Error',
122+
value: 'Some error',
123+
}),
124+
);
125+
expect(event.extra).toEqual({
126+
__serialized__: {
127+
foo: { bar: 'baz' },
128+
name: 'BadType',
129+
err: {
130+
message: 'Some error',
131+
name: 'Error',
132+
stack: expect.stringContaining('Error: Some error'),
133+
},
134+
},
135+
});
136+
});
86137
});

‎packages/utils/src/eventbuilder.ts‎

Lines changed: 78 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type {
1111
StackParser,
1212
} from '@sentry/types';
1313

14-
import { isError, isParameterizedString, isPlainObject } from './is';
14+
import { isError, isErrorEvent, isParameterizedString, isPlainObject } from './is';
1515
import { addExceptionMechanism, addExceptionTypeValue } from './misc';
1616
import { normalizeToSize } from './normalize';
1717
import { extractExceptionKeysForMessage } from './object';
@@ -40,7 +40,21 @@ export function exceptionFromError(stackParser: StackParser, error: Error): Exce
4040
return exception;
4141
}
4242

43-
function getMessageForObject(exception: object): string {
43+
/** If a plain object has a property that is an `Error`, return this error. */
44+
function getErrorPropertyFromObject(obj: Record<string, unknown>): Error | undefined {
45+
for (const prop in obj) {
46+
if (Object.prototype.hasOwnProperty.call(obj, prop)) {
47+
const value = obj[prop];
48+
if (value instanceof Error) {
49+
return value;
50+
}
51+
}
52+
}
53+
54+
return undefined;
55+
}
56+
57+
function getMessageForObject(exception: Record<string, unknown>): string {
4458
if ('name' in exception && typeof exception.name === 'string') {
4559
let message = `'${exception.name}' captured as exception`;
4660

@@ -51,13 +65,67 @@ function getMessageForObject(exception: object): string {
5165
return message;
5266
} else if ('message' in exception && typeof exception.message === 'string') {
5367
return exception.message;
54-
} else {
55-
// This will allow us to group events based on top-level keys
56-
// which is much better than creating new group when any key/value change
57-
return `Object captured as exception with keys: ${extractExceptionKeysForMessage(
58-
exception as Record<string, unknown>,
59-
)}`;
6068
}
69+
70+
const keys = extractExceptionKeysForMessage(exception);
71+
72+
// Some ErrorEvent instances do not have an `error` property, which is why they are not handled before
73+
// We still want to try to get a decent message for these cases
74+
if (isErrorEvent(exception)) {
75+
return `Event \`ErrorEvent\` captured as exception with message \`${exception.message}\``;
76+
}
77+
78+
const className = getObjectClassName(exception);
79+
80+
return `${
81+
className && className !== 'Object' ? `'${className}'` : 'Object'
82+
} captured as exception with keys: ${keys}`;
83+
}
84+
85+
function getObjectClassName(obj: unknown): string | undefined | void {
86+
try {
87+
const prototype: unknown | null = Object.getPrototypeOf(obj);
88+
return prototype ? prototype.constructor.name : undefined;
89+
} catch (e) {
90+
// ignore errors here
91+
}
92+
}
93+
94+
function getException(
95+
client: Client,
96+
mechanism: Mechanism,
97+
exception: unknown,
98+
hint?: EventHint,
99+
): [Error, Extras | undefined] {
100+
if (isError(exception)) {
101+
return [exception, undefined];
102+
}
103+
104+
// Mutate this!
105+
mechanism.synthetic = true;
106+
107+
if (isPlainObject(exception)) {
108+
const normalizeDepth = client && client.getOptions().normalizeDepth;
109+
const extras = { ['__serialized__']: normalizeToSize(exception as Record<string, unknown>, normalizeDepth) };
110+
111+
const errorFromProp = getErrorPropertyFromObject(exception);
112+
if (errorFromProp) {
113+
return [errorFromProp, extras];
114+
}
115+
116+
const message = getMessageForObject(exception);
117+
const ex = (hint && hint.syntheticException) || new Error(message);
118+
ex.message = message;
119+
120+
return [ex, extras];
121+
}
122+
123+
// This handles when someone does: `throw "something awesome";`
124+
// We use synthesized Error here so we can extract a (rough) stack trace.
125+
const ex = (hint && hint.syntheticException) || new Error(exception as string);
126+
ex.message = `${exception}`;
127+
128+
return [ex, undefined];
61129
}
62130

63131
/**
@@ -70,36 +138,18 @@ export function eventFromUnknownInput(
70138
exception: unknown,
71139
hint?: EventHint,
72140
): Event {
73-
let ex: unknown = exception;
74141
const providedMechanism: Mechanism | undefined =
75142
hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism;
76143
const mechanism: Mechanism = providedMechanism || {
77144
handled: true,
78145
type: 'generic',
79146
};
80147

81-
let extras: Extras | undefined;
82-
83-
if (!isError(exception)) {
84-
if (isPlainObject(exception)) {
85-
const normalizeDepth = client && client.getOptions().normalizeDepth;
86-
extras = { ['__serialized__']: normalizeToSize(exception as Record<string, unknown>, normalizeDepth) };
87-
88-
const message = getMessageForObject(exception);
89-
ex = (hint && hint.syntheticException) || new Error(message);
90-
(ex as Error).message = message;
91-
} else {
92-
// This handles when someone does: `throw "something awesome";`
93-
// We use synthesized Error here so we can extract a (rough) stack trace.
94-
ex = (hint && hint.syntheticException) || new Error(exception as string);
95-
(ex as Error).message = exception as string;
96-
}
97-
mechanism.synthetic = true;
98-
}
148+
const [ex, extras] = getException(client, mechanism, exception, hint);
99149

100150
const event: Event = {
101151
exception: {
102-
values: [exceptionFromError(stackParser, ex as Error)],
152+
values: [exceptionFromError(stackParser, ex)],
103153
},
104154
};
105155

0 commit comments

Comments
 (0)