Skip to content

Commit 5bc9a38

Browse files
authored
fix(utils): Update eventFromUnknownInput to avoid scope pollution & getCurrentHub (#9868)
Instead we can pass a client directly, and I refactored the method to avoid setting extra on the scope, and just set it on the event directly - as this is also kind of leaking right now, because the extra may also be applied to other events using the same scope.
1 parent ffc4181 commit 5bc9a38

File tree

5 files changed

+79
-42
lines changed

5 files changed

+79
-42
lines changed

packages/core/src/server-runtime-client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { eventFromMessage, eventFromUnknownInput, logger, resolvedSyncPromise, u
1616
import { BaseClient } from './baseclient';
1717
import { createCheckInEnvelope } from './checkin';
1818
import { DEBUG_BUILD } from './debug-build';
19-
import { getCurrentHub } from './hub';
19+
import { getClient } from './exports';
2020
import type { Scope } from './scope';
2121
import { SessionFlusher } from './sessionflusher';
2222
import { addTracingExtensions, getDynamicSamplingContextFromClient } from './tracing';
@@ -50,7 +50,7 @@ export class ServerRuntimeClient<
5050
* @inheritDoc
5151
*/
5252
public eventFromException(exception: unknown, hint?: EventHint): PromiseLike<Event> {
53-
return resolvedSyncPromise(eventFromUnknownInput(getCurrentHub, this._options.stackParser, exception, hint));
53+
return resolvedSyncPromise(eventFromUnknownInput(getClient(), this._options.stackParser, exception, hint));
5454
}
5555

5656
/**

packages/deno/src/integrations/globalhandlers.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ServerRuntimeClient } from '@sentry/core';
2-
import { flush, getCurrentHub } from '@sentry/core';
2+
import { getClient, getCurrentHub, getCurrentScope } from '@sentry/core';
3+
import { flush } from '@sentry/core';
34
import type { Event, Hub, Integration, Primitive, StackParser } from '@sentry/types';
45
import { eventFromUnknownInput, isPrimitive } from '@sentry/utils';
56

@@ -70,7 +71,7 @@ function installGlobalErrorHandler(): void {
7071
const [hub, stackParser] = getHubAndOptions();
7172
const { message, error } = data;
7273

73-
const event = eventFromUnknownInput(getCurrentHub, stackParser, error || message);
74+
const event = eventFromUnknownInput(getClient(), stackParser, error || message);
7475

7576
event.level = 'fatal';
7677

@@ -113,7 +114,7 @@ function installGlobalUnhandledRejectionHandler(): void {
113114

114115
const event = isPrimitive(error)
115116
? eventFromRejectionWithPrimitive(error)
116-
: eventFromUnknownInput(getCurrentHub, stackParser, error, undefined);
117+
: eventFromUnknownInput(getClient(), stackParser, error, undefined);
117118

118119
event.level = 'fatal';
119120

packages/node/test/eventbuilders.test.ts

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,51 @@
1-
import type { Client } from '@sentry/types';
1+
import type { Hub } from '@sentry/types';
22
import { eventFromUnknownInput } from '@sentry/utils';
33

4-
import { Scope, defaultStackParser, getCurrentHub } from '../src';
4+
import { defaultStackParser } from '../src';
55

6-
const testScope = new Scope();
7-
8-
jest.mock('@sentry/core', () => {
9-
const original = jest.requireActual('@sentry/core');
10-
return {
11-
...original,
12-
getCurrentHub(): {
13-
getClient(): Client;
14-
getScope(): Scope;
15-
} {
16-
return {
17-
getClient(): any {
18-
return {
19-
getOptions(): any {
20-
return { normalizeDepth: 6 };
6+
describe('eventFromUnknownInput', () => {
7+
test('uses normalizeDepth from init options', () => {
8+
const deepObject = {
9+
a: {
10+
b: {
11+
c: {
12+
d: {
13+
e: {
14+
f: {
15+
g: 'foo',
16+
},
17+
},
2118
},
22-
};
23-
},
24-
getScope(): Scope {
25-
return testScope;
19+
},
2620
},
27-
};
28-
},
29-
};
30-
});
21+
},
22+
};
3123

32-
afterEach(() => {
33-
jest.resetAllMocks();
34-
});
24+
const client = {
25+
getOptions(): any {
26+
return { normalizeDepth: 6 };
27+
},
28+
} as any;
29+
const event = eventFromUnknownInput(client, defaultStackParser, deepObject);
3530

36-
describe('eventFromUnknownInput', () => {
37-
test('uses normalizeDepth from init options', () => {
31+
const serializedObject = event.extra?.__serialized__;
32+
expect(serializedObject).toBeDefined();
33+
expect(serializedObject).toEqual({
34+
a: {
35+
b: {
36+
c: {
37+
d: {
38+
e: {
39+
f: '[Object]',
40+
},
41+
},
42+
},
43+
},
44+
},
45+
});
46+
});
47+
48+
test('uses normalizeDepth from init options (passing getCurrentHub)', () => {
3849
const deepObject = {
3950
a: {
4051
b: {
@@ -51,9 +62,19 @@ describe('eventFromUnknownInput', () => {
5162
},
5263
};
5364

54-
eventFromUnknownInput(getCurrentHub, defaultStackParser, deepObject);
65+
const getCurrentHub = jest.fn(() => {
66+
return {
67+
getClient: () => ({
68+
getOptions(): any {
69+
return { normalizeDepth: 6 };
70+
},
71+
}),
72+
} as unknown as Hub;
73+
});
74+
75+
const event = eventFromUnknownInput(getCurrentHub, defaultStackParser, deepObject);
5576

56-
const serializedObject = (testScope as any)._extra.__serialized__;
77+
const serializedObject = event.extra?.__serialized__;
5778
expect(serializedObject).toBeDefined();
5879
expect(serializedObject).toEqual({
5980
a: {

packages/utils/src/eventbuilder.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import type {
2+
Client,
23
Event,
34
EventHint,
45
Exception,
6+
Extras,
57
Hub,
68
Mechanism,
79
Severity,
@@ -61,14 +63,18 @@ function getMessageForObject(exception: object): string {
6163

6264
/**
6365
* Builds and Event from a Exception
66+
*
67+
* TODO(v8): Remove getHub fallback
6468
* @hidden
6569
*/
6670
export function eventFromUnknownInput(
67-
getCurrentHub: () => Hub,
71+
getHubOrClient: (() => Hub) | Client | undefined,
6872
stackParser: StackParser,
6973
exception: unknown,
7074
hint?: EventHint,
7175
): Event {
76+
const client = typeof getHubOrClient === 'function' ? getHubOrClient().getClient() : getHubOrClient;
77+
7278
let ex: unknown = exception;
7379
const providedMechanism: Mechanism | undefined =
7480
hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism;
@@ -77,12 +83,12 @@ export function eventFromUnknownInput(
7783
type: 'generic',
7884
};
7985

86+
let extras: Extras | undefined;
87+
8088
if (!isError(exception)) {
8189
if (isPlainObject(exception)) {
82-
const hub = getCurrentHub();
83-
const client = hub.getClient();
8490
const normalizeDepth = client && client.getOptions().normalizeDepth;
85-
hub.getScope().setExtra('__serialized__', normalizeToSize(exception, normalizeDepth));
91+
extras = { ['__serialized__']: normalizeToSize(exception as Record<string, unknown>, normalizeDepth) };
8692

8793
const message = getMessageForObject(exception);
8894
ex = (hint && hint.syntheticException) || new Error(message);
@@ -96,12 +102,16 @@ export function eventFromUnknownInput(
96102
mechanism.synthetic = true;
97103
}
98104

99-
const event = {
105+
const event: Event = {
100106
exception: {
101107
values: [exceptionFromError(stackParser, ex as Error)],
102108
},
103109
};
104110

111+
if (extras) {
112+
event.extra = extras;
113+
}
114+
105115
addExceptionTypeValue(event, undefined, undefined);
106116
addExceptionMechanism(event, mechanism);
107117

packages/utils/test/eventbuilder.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,9 @@ describe('eventFromUnknownInput', () => {
3636
const event = eventFromUnknownInput(getCurrentHub, stackParser, { foo: { bar: 'baz' }, message: 'Some message' });
3737
expect(event.exception?.values?.[0].value).toBe('Some message');
3838
});
39+
40+
test('passing client directly', () => {
41+
const event = eventFromUnknownInput(undefined, stackParser, { foo: { bar: 'baz' }, prop: 1 });
42+
expect(event.exception?.values?.[0].value).toBe('Object captured as exception with keys: foo, prop');
43+
});
3944
});

0 commit comments

Comments
 (0)