Skip to content

Commit f251833

Browse files
authored
feat(core): Allow to pass mechanism as event hint (#9590)
This allows to pass `mechanism` as an event hint. Also, we now allow to pass `hint` as an alternative to `CaptureContext` to `captureException` as second argument. We have quite a lot of code where we fork a scope and add an event processor, only to add a mechanism to the event. Since this is a quite common pattern, I figured it makes more sense to allow to pass a mechanism as an EventHint. In addition, I noticed that while `hub.captureException()` takes an event hint as second argument, in the core exported `captureException()` we take a `CaptureContext` as second argument instead (for legacy reasons). In order to be able to pass a mechanism there as well, I updated the method signature to allow _either_ a CaptureContext _or_ an EventHint. I wrote some tests covering this to make sure that works - it's a bit tricky since both can be POJOs, but no fields overlap so we are able to parse this together.
1 parent 13b4b47 commit f251833

38 files changed

+434
-631
lines changed

packages/angular/src/errorhandler.ts

+4-13
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import { HttpErrorResponse } from '@angular/common/http';
22
import type { ErrorHandler as AngularErrorHandler } from '@angular/core';
33
import { Inject, Injectable } from '@angular/core';
44
import * as Sentry from '@sentry/browser';
5-
import type { Event, Scope } from '@sentry/types';
6-
import { addExceptionMechanism, isString } from '@sentry/utils';
5+
import type { Event } from '@sentry/types';
6+
import { isString } from '@sentry/utils';
77

88
import { runOutsideAngular } from './zone';
99

@@ -102,17 +102,8 @@ class SentryErrorHandler implements AngularErrorHandler {
102102

103103
// Capture handled exception and send it to Sentry.
104104
const eventId = runOutsideAngular(() =>
105-
Sentry.captureException(extractedError, (scope: Scope) => {
106-
scope.addEventProcessor(event => {
107-
addExceptionMechanism(event, {
108-
type: 'angular',
109-
handled: false,
110-
});
111-
112-
return event;
113-
});
114-
115-
return scope;
105+
Sentry.captureException(extractedError, {
106+
mechanism: { type: 'angular', handled: false },
116107
}),
117108
);
118109

packages/angular/test/errorhandler.test.ts

+43-70
Large diffs are not rendered by default.

packages/astro/src/server/middleware.ts

+9-14
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { captureException, configureScope, getCurrentHub, startSpan } from '@sentry/node';
22
import type { Hub, Span } from '@sentry/types';
3-
import { addExceptionMechanism, objectify, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
3+
import { objectify, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
44
import type { APIContext, MiddlewareResponseHandler } from 'astro';
55

66
import { getTracingMetaTags } from './meta';
@@ -34,19 +34,14 @@ function sendErrorToSentry(e: unknown): unknown {
3434
// store a seen flag on it.
3535
const objectifiedErr = objectify(e);
3636

37-
captureException(objectifiedErr, scope => {
38-
scope.addEventProcessor(event => {
39-
addExceptionMechanism(event, {
40-
type: 'astro',
41-
handled: false,
42-
data: {
43-
function: 'astroMiddleware',
44-
},
45-
});
46-
return event;
47-
});
48-
49-
return scope;
37+
captureException(objectifiedErr, {
38+
mechanism: {
39+
type: 'astro',
40+
handled: false,
41+
data: {
42+
function: 'astroMiddleware',
43+
},
44+
},
5045
});
5146

5247
return objectifiedErr;

packages/astro/test/server/middleware.test.ts

+4-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as SentryNode from '@sentry/node';
2-
import * as SentryUtils from '@sentry/utils';
32
import { vi } from 'vitest';
43

54
import { handleRequest, interpolateRouteFromUrlAndParams } from '../../src/server/middleware';
@@ -59,12 +58,7 @@ describe('sentryMiddleware', () => {
5958
});
6059

6160
it('throws and sends an error to sentry if `next()` throws', async () => {
62-
const scope = {
63-
addEventProcessor: vi.fn().mockImplementation(cb => cb({})),
64-
};
65-
// @ts-expect-error, just testing the callback, this is okay for this test
66-
const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException').mockImplementation((ex, cb) => cb(scope));
67-
const addExMechanismSpy = vi.spyOn(SentryUtils, 'addExceptionMechanism');
61+
const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException');
6862

6963
const middleware = handleRequest();
7064
const ctx = {
@@ -86,16 +80,9 @@ describe('sentryMiddleware', () => {
8680
// @ts-expect-error, a partial ctx object is fine here
8781
await expect(async () => middleware(ctx, next)).rejects.toThrowError();
8882

89-
expect(captureExceptionSpy).toHaveBeenCalledWith(error, expect.any(Function));
90-
expect(scope.addEventProcessor).toHaveBeenCalledTimes(1);
91-
expect(addExMechanismSpy).toHaveBeenCalledWith(
92-
{}, // the mocked event
93-
{
94-
handled: false,
95-
type: 'astro',
96-
data: { function: 'astroMiddleware' },
97-
},
98-
);
83+
expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
84+
mechanism: { handled: false, type: 'astro', data: { function: 'astroMiddleware' } },
85+
});
9986
});
10087

10188
it('attaches tracing headers', async () => {

packages/browser/src/helpers.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { captureException, withScope } from '@sentry/core';
2-
import type { DsnLike, Event as SentryEvent, Mechanism, Scope, WrappedFunction } from '@sentry/types';
2+
import type { DsnLike, Mechanism, WrappedFunction } from '@sentry/types';
33
import {
44
addExceptionMechanism,
55
addExceptionTypeValue,
@@ -99,8 +99,8 @@ export function wrap(
9999
} catch (ex) {
100100
ignoreNextOnError();
101101

102-
withScope((scope: Scope) => {
103-
scope.addEventProcessor((event: SentryEvent) => {
102+
withScope(scope => {
103+
scope.addEventProcessor(event => {
104104
if (options.mechanism) {
105105
addExceptionTypeValue(event, undefined, undefined);
106106
addExceptionMechanism(event, options.mechanism);

packages/browser/src/integrations/globalhandlers.ts

+17-22
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,7 @@
11
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
22
import { getCurrentHub } from '@sentry/core';
3-
import type { Event, EventHint, Hub, Integration, Primitive, StackParser } from '@sentry/types';
4-
import {
5-
addExceptionMechanism,
6-
addInstrumentationHandler,
7-
getLocationHref,
8-
isErrorEvent,
9-
isPrimitive,
10-
isString,
11-
logger,
12-
} from '@sentry/utils';
3+
import type { Event, Hub, Integration, Primitive, StackParser } from '@sentry/types';
4+
import { addInstrumentationHandler, getLocationHref, isErrorEvent, isPrimitive, isString, logger } from '@sentry/utils';
135

146
import type { BrowserClient } from '../client';
157
import { eventFromUnknownInput } from '../eventbuilder';
@@ -103,7 +95,13 @@ function _installGlobalOnErrorHandler(): void {
10395

10496
event.level = 'error';
10597

106-
addMechanismAndCapture(hub, error, event, 'onerror');
98+
hub.captureEvent(event, {
99+
originalException: error,
100+
mechanism: {
101+
handled: false,
102+
type: 'onerror',
103+
},
104+
});
107105
},
108106
);
109107
}
@@ -149,7 +147,14 @@ function _installGlobalOnUnhandledRejectionHandler(): void {
149147

150148
event.level = 'error';
151149

152-
addMechanismAndCapture(hub, error, event, 'onunhandledrejection');
150+
hub.captureEvent(event, {
151+
originalException: error,
152+
mechanism: {
153+
handled: false,
154+
type: 'onunhandledrejection',
155+
},
156+
});
157+
153158
return;
154159
},
155160
);
@@ -243,16 +248,6 @@ function globalHandlerLog(type: string): void {
243248
__DEBUG_BUILD__ && logger.log(`Global Handler attached: ${type}`);
244249
}
245250

246-
function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void {
247-
addExceptionMechanism(event, {
248-
handled: false,
249-
type,
250-
});
251-
hub.captureEvent(event, {
252-
originalException: error,
253-
});
254-
}
255-
256251
function getHubAndOptions(): [Hub, StackParser, boolean | undefined] {
257252
const hub = getCurrentHub();
258253
const client = hub.getClient<BrowserClient>();

packages/bun/src/integrations/bunserver.ts

+10-21
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,6 @@
11
import { captureException, continueTrace, runWithAsyncContext, startSpan, Transaction } from '@sentry/core';
22
import type { Integration } from '@sentry/types';
3-
import { addExceptionMechanism, getSanitizedUrlString, parseUrl } from '@sentry/utils';
4-
5-
function sendErrorToSentry(e: unknown): unknown {
6-
captureException(e, scope => {
7-
scope.addEventProcessor(event => {
8-
addExceptionMechanism(event, {
9-
type: 'bun',
10-
handled: false,
11-
data: {
12-
function: 'serve',
13-
},
14-
});
15-
return event;
16-
});
17-
18-
return scope;
19-
});
20-
21-
return e;
22-
}
3+
import { getSanitizedUrlString, parseUrl } from '@sentry/utils';
234

245
/**
256
* Instruments `Bun.serve` to automatically create transactions and capture errors.
@@ -115,7 +96,15 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
11596
}
11697
return response;
11798
} catch (e) {
118-
sendErrorToSentry(e);
99+
captureException(e, {
100+
mechanism: {
101+
type: 'bun',
102+
handled: false,
103+
data: {
104+
function: 'serve',
105+
},
106+
},
107+
});
119108
throw e;
120109
}
121110
},

packages/core/src/exports.ts

+10-7
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import { isThenable, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
2020
import type { Hub } from './hub';
2121
import { getCurrentHub } from './hub';
2222
import type { Scope } from './scope';
23+
import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent';
24+
import { parseEventHintOrCaptureContext } from './utils/prepareEvent';
2325

2426
// Note: All functions in this file are typed with a return value of `ReturnType<Hub[HUB_FUNCTION]>`,
2527
// where HUB_FUNCTION is some method on the Hub class.
@@ -30,14 +32,15 @@ import type { Scope } from './scope';
3032

3133
/**
3234
* Captures an exception event and sends it to Sentry.
33-
*
34-
* @param exception An exception-like object.
35-
* @param captureContext Additional scope data to apply to exception event.
36-
* @returns The generated eventId.
35+
* This accepts an event hint as optional second parameter.
36+
* Alternatively, you can also pass a CaptureContext directly as second parameter.
3737
*/
38-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
39-
export function captureException(exception: any, captureContext?: CaptureContext): ReturnType<Hub['captureException']> {
40-
return getCurrentHub().captureException(exception, { captureContext });
38+
export function captureException(
39+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
40+
exception: any,
41+
hint?: ExclusiveEventHintOrCaptureContext,
42+
): ReturnType<Hub['captureException']> {
43+
return getCurrentHub().captureException(exception, parseEventHintOrCaptureContext(hint));
4144
}
4245

4346
/**

packages/core/src/utils/prepareEvent.ts

+80-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,37 @@
1-
import type { Client, ClientOptions, Event, EventHint, StackFrame, StackParser } from '@sentry/types';
2-
import { dateTimestampInSeconds, GLOBAL_OBJ, normalize, resolvedSyncPromise, truncate, uuid4 } from '@sentry/utils';
1+
import type {
2+
CaptureContext,
3+
Client,
4+
ClientOptions,
5+
Event,
6+
EventHint,
7+
Scope as ScopeInterface,
8+
ScopeContext,
9+
StackFrame,
10+
StackParser,
11+
} from '@sentry/types';
12+
import {
13+
addExceptionMechanism,
14+
dateTimestampInSeconds,
15+
GLOBAL_OBJ,
16+
normalize,
17+
resolvedSyncPromise,
18+
truncate,
19+
uuid4,
20+
} from '@sentry/utils';
321

422
import { DEFAULT_ENVIRONMENT } from '../constants';
523
import { getGlobalEventProcessors, notifyEventProcessors } from '../eventProcessors';
624
import { Scope } from '../scope';
725

26+
/**
27+
* This type makes sure that we get either a CaptureContext, OR an EventHint.
28+
* It does not allow mixing them, which could lead to unexpected outcomes, e.g. this is disallowed:
29+
* { user: { id: '123' }, mechanism: { handled: false } }
30+
*/
31+
export type ExclusiveEventHintOrCaptureContext =
32+
| (CaptureContext & Partial<{ [key in keyof EventHint]: never }>)
33+
| (EventHint & Partial<{ [key in keyof ScopeContext]: never }>);
34+
835
/**
936
* Adds common information to events.
1037
*
@@ -52,6 +79,10 @@ export function prepareEvent(
5279
finalScope = Scope.clone(finalScope).update(hint.captureContext);
5380
}
5481

82+
if (hint.mechanism) {
83+
addExceptionMechanism(prepared, hint.mechanism);
84+
}
85+
5586
// We prepare the result here with a resolved Event.
5687
let result = resolvedSyncPromise<Event | null>(prepared);
5788

@@ -309,3 +340,50 @@ function normalizeEvent(event: Event | null, depth: number, maxBreadth: number):
309340

310341
return normalized;
311342
}
343+
344+
/**
345+
* Parse either an `EventHint` directly, or convert a `CaptureContext` to an `EventHint`.
346+
* This is used to allow to update method signatures that used to accept a `CaptureContext` but should now accept an `EventHint`.
347+
*/
348+
export function parseEventHintOrCaptureContext(
349+
hint: ExclusiveEventHintOrCaptureContext | undefined,
350+
): EventHint | undefined {
351+
if (!hint) {
352+
return undefined;
353+
}
354+
355+
// If you pass a Scope or `() => Scope` as CaptureContext, we just return this as captureContext
356+
if (hintIsScopeOrFunction(hint)) {
357+
return { captureContext: hint };
358+
}
359+
360+
if (hintIsScopeContext(hint)) {
361+
return {
362+
captureContext: hint,
363+
};
364+
}
365+
366+
return hint;
367+
}
368+
369+
function hintIsScopeOrFunction(
370+
hint: CaptureContext | EventHint,
371+
): hint is ScopeInterface | ((scope: ScopeInterface) => ScopeInterface) {
372+
return hint instanceof Scope || typeof hint === 'function';
373+
}
374+
375+
type ScopeContextProperty = keyof ScopeContext;
376+
const captureContextKeys: readonly ScopeContextProperty[] = [
377+
'user',
378+
'level',
379+
'extra',
380+
'contexts',
381+
'tags',
382+
'fingerprint',
383+
'requestSession',
384+
'propagationContext',
385+
] as const;
386+
387+
function hintIsScopeContext(hint: Partial<ScopeContext> | EventHint): hint is Partial<ScopeContext> {
388+
return Object.keys(hint).some(key => captureContextKeys.includes(key as ScopeContextProperty));
389+
}

0 commit comments

Comments
 (0)