Skip to content

feat(core): Allow to pass mechanism as event hint #9590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions packages/angular/src/errorhandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { HttpErrorResponse } from '@angular/common/http';
import type { ErrorHandler as AngularErrorHandler } from '@angular/core';
import { Inject, Injectable } from '@angular/core';
import * as Sentry from '@sentry/browser';
import type { Event, Scope } from '@sentry/types';
import { addExceptionMechanism, isString } from '@sentry/utils';
import type { Event } from '@sentry/types';
import { isString } from '@sentry/utils';

import { runOutsideAngular } from './zone';

Expand Down Expand Up @@ -102,17 +102,8 @@ class SentryErrorHandler implements AngularErrorHandler {

// Capture handled exception and send it to Sentry.
const eventId = runOutsideAngular(() =>
Sentry.captureException(extractedError, (scope: Scope) => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
type: 'angular',
handled: false,
});

return event;
});

return scope;
Sentry.captureException(extractedError, {
mechanism: { type: 'angular', handled: false },
}),
);

Expand Down
113 changes: 43 additions & 70 deletions packages/angular/test/errorhandler.test.ts

Large diffs are not rendered by default.

23 changes: 9 additions & 14 deletions packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { captureException, configureScope, getCurrentHub, startSpan } from '@sentry/node';
import type { Hub, Span } from '@sentry/types';
import { addExceptionMechanism, objectify, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
import { objectify, stripUrlQueryAndFragment, tracingContextFromHeaders } from '@sentry/utils';
import type { APIContext, MiddlewareResponseHandler } from 'astro';

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

captureException(objectifiedErr, scope => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
type: 'astro',
handled: false,
data: {
function: 'astroMiddleware',
},
});
return event;
});

return scope;
captureException(objectifiedErr, {
mechanism: {
type: 'astro',
handled: false,
data: {
function: 'astroMiddleware',
},
},
});

return objectifiedErr;
Expand Down
21 changes: 4 additions & 17 deletions packages/astro/test/server/middleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as SentryNode from '@sentry/node';
import * as SentryUtils from '@sentry/utils';
import { vi } from 'vitest';

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

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

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

expect(captureExceptionSpy).toHaveBeenCalledWith(error, expect.any(Function));
expect(scope.addEventProcessor).toHaveBeenCalledTimes(1);
expect(addExMechanismSpy).toHaveBeenCalledWith(
{}, // the mocked event
{
handled: false,
type: 'astro',
data: { function: 'astroMiddleware' },
},
);
expect(captureExceptionSpy).toHaveBeenCalledWith(error, {
mechanism: { handled: false, type: 'astro', data: { function: 'astroMiddleware' } },
});
});

it('attaches tracing headers', async () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { captureException, withScope } from '@sentry/core';
import type { DsnLike, Event as SentryEvent, Mechanism, Scope, WrappedFunction } from '@sentry/types';
import type { DsnLike, Mechanism, WrappedFunction } from '@sentry/types';
import {
addExceptionMechanism,
addExceptionTypeValue,
Expand Down Expand Up @@ -99,8 +99,8 @@ export function wrap(
} catch (ex) {
ignoreNextOnError();

withScope((scope: Scope) => {
scope.addEventProcessor((event: SentryEvent) => {
withScope(scope => {
scope.addEventProcessor(event => {
if (options.mechanism) {
addExceptionTypeValue(event, undefined, undefined);
addExceptionMechanism(event, options.mechanism);
Expand Down
39 changes: 17 additions & 22 deletions packages/browser/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
import { getCurrentHub } from '@sentry/core';
import type { Event, EventHint, Hub, Integration, Primitive, StackParser } from '@sentry/types';
import {
addExceptionMechanism,
addInstrumentationHandler,
getLocationHref,
isErrorEvent,
isPrimitive,
isString,
logger,
} from '@sentry/utils';
import type { Event, Hub, Integration, Primitive, StackParser } from '@sentry/types';
import { addInstrumentationHandler, getLocationHref, isErrorEvent, isPrimitive, isString, logger } from '@sentry/utils';

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

event.level = 'error';

addMechanismAndCapture(hub, error, event, 'onerror');
hub.captureEvent(event, {
originalException: error,
mechanism: {
handled: false,
type: 'onerror',
},
});
},
);
}
Expand Down Expand Up @@ -149,7 +147,14 @@ function _installGlobalOnUnhandledRejectionHandler(): void {

event.level = 'error';

addMechanismAndCapture(hub, error, event, 'onunhandledrejection');
hub.captureEvent(event, {
originalException: error,
mechanism: {
handled: false,
type: 'onunhandledrejection',
},
});

return;
},
);
Expand Down Expand Up @@ -243,16 +248,6 @@ function globalHandlerLog(type: string): void {
__DEBUG_BUILD__ && logger.log(`Global Handler attached: ${type}`);
}

function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void {
addExceptionMechanism(event, {
handled: false,
type,
});
hub.captureEvent(event, {
originalException: error,
});
}

function getHubAndOptions(): [Hub, StackParser, boolean | undefined] {
const hub = getCurrentHub();
const client = hub.getClient<BrowserClient>();
Expand Down
31 changes: 10 additions & 21 deletions packages/bun/src/integrations/bunserver.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,6 @@
import { captureException, continueTrace, runWithAsyncContext, startSpan, Transaction } from '@sentry/core';
import type { Integration } from '@sentry/types';
import { addExceptionMechanism, getSanitizedUrlString, parseUrl } from '@sentry/utils';

function sendErrorToSentry(e: unknown): unknown {
captureException(e, scope => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
type: 'bun',
handled: false,
data: {
function: 'serve',
},
});
return event;
});

return scope;
});

return e;
}
import { getSanitizedUrlString, parseUrl } from '@sentry/utils';

/**
* Instruments `Bun.serve` to automatically create transactions and capture errors.
Expand Down Expand Up @@ -115,7 +96,15 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
}
return response;
} catch (e) {
sendErrorToSentry(e);
captureException(e, {
mechanism: {
type: 'bun',
handled: false,
data: {
function: 'serve',
},
},
});
throw e;
}
},
Expand Down
17 changes: 10 additions & 7 deletions packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { isThenable, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
import type { Hub } from './hub';
import { getCurrentHub } from './hub';
import type { Scope } from './scope';
import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent';
import { parseEventHintOrCaptureContext } from './utils/prepareEvent';

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

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

/**
Expand Down
82 changes: 80 additions & 2 deletions packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,37 @@
import type { Client, ClientOptions, Event, EventHint, StackFrame, StackParser } from '@sentry/types';
import { dateTimestampInSeconds, GLOBAL_OBJ, normalize, resolvedSyncPromise, truncate, uuid4 } from '@sentry/utils';
import type {
CaptureContext,
Client,
ClientOptions,
Event,
EventHint,
Scope as ScopeInterface,
ScopeContext,
StackFrame,
StackParser,
} from '@sentry/types';
import {
addExceptionMechanism,
dateTimestampInSeconds,
GLOBAL_OBJ,
normalize,
resolvedSyncPromise,
truncate,
uuid4,
} from '@sentry/utils';

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

/**
* This type makes sure that we get either a CaptureContext, OR an EventHint.
* It does not allow mixing them, which could lead to unexpected outcomes, e.g. this is disallowed:
* { user: { id: '123' }, mechanism: { handled: false } }
*/
export type ExclusiveEventHintOrCaptureContext =
| (CaptureContext & Partial<{ [key in keyof EventHint]: never }>)
| (EventHint & Partial<{ [key in keyof ScopeContext]: never }>);

/**
* Adds common information to events.
*
Expand Down Expand Up @@ -52,6 +79,10 @@ export function prepareEvent(
finalScope = Scope.clone(finalScope).update(hint.captureContext);
}

if (hint.mechanism) {
addExceptionMechanism(prepared, hint.mechanism);
}

// We prepare the result here with a resolved Event.
let result = resolvedSyncPromise<Event | null>(prepared);

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

return normalized;
}

/**
* Parse either an `EventHint` directly, or convert a `CaptureContext` to an `EventHint`.
* This is used to allow to update method signatures that used to accept a `CaptureContext` but should now accept an `EventHint`.
*/
export function parseEventHintOrCaptureContext(
hint: ExclusiveEventHintOrCaptureContext | undefined,
): EventHint | undefined {
if (!hint) {
return undefined;
}

// If you pass a Scope or `() => Scope` as CaptureContext, we just return this as captureContext
if (hintIsScopeOrFunction(hint)) {
return { captureContext: hint };
}

if (hintIsScopeContext(hint)) {
return {
captureContext: hint,
};
}

return hint;
}

function hintIsScopeOrFunction(
hint: CaptureContext | EventHint,
): hint is ScopeInterface | ((scope: ScopeInterface) => ScopeInterface) {
return hint instanceof Scope || typeof hint === 'function';
}

type ScopeContextProperty = keyof ScopeContext;
const captureContextKeys: readonly ScopeContextProperty[] = [
'user',
'level',
'extra',
'contexts',
'tags',
'fingerprint',
'requestSession',
'propagationContext',
] as const;

function hintIsScopeContext(hint: Partial<ScopeContext> | EventHint): hint is Partial<ScopeContext> {
return Object.keys(hint).some(key => captureContextKeys.includes(key as ScopeContextProperty));
}
Loading