From f4418aef8d1d4630f5d2afd5b00d8d6a60cda15f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 14 Oct 2021 18:40:50 -0700 Subject: [PATCH 1/7] add docstrings to Mechanism type --- packages/types/src/mechanism.ts | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/types/src/mechanism.ts b/packages/types/src/mechanism.ts index b1164cfcd93a..ee90d2eda762 100644 --- a/packages/types/src/mechanism.ts +++ b/packages/types/src/mechanism.ts @@ -1,9 +1,32 @@ -/** JSDoc */ +/** + * Metadata about a captured exception, intended to provide a hint as to the means by which it was captured. + */ export interface Mechanism { + /** + * For now, restricted to `onerror`, `onunhandledrejection` (both obvious), `instrument` (the result of + * auto-instrumentation), and `generic` (everything else). Converted to a tag on ingest. + */ type: string; + + /** + * In theory, whether or not the exception has been handled by the user. In practice, whether or not we see it before + * it hits the global error/rejection handlers, whether through explicit handling by the user or auto instrumentation. + * Converted to a tag on ingest and used in various ways in the UI. + */ handled: boolean; + + /** + * Arbitrary data to be associated with the mechanism (for example, errors coming from event handlers include the + * handler name and the event target. Will show up in the UI directly above the stacktrace. + */ data?: { [key: string]: string | boolean; }; + + /** + * True when `captureException` is called with anything other than an instance of `Error` (or, in the case of browser, + * an instance of `ErrorEvent`, `DOMError`, or `DOMException`). causing us to create a synthetic error in an attempt + * to recreate the stacktrace. + */ synthetic?: boolean; } From c49257c2422c7268df81187c8097bf2200c05749 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 13 Oct 2021 23:14:30 -0700 Subject: [PATCH 2/7] use actual Mechanisms in addExceptionMechanism --- packages/utils/src/misc.ts | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 2793d24bc585..a40ac5d01547 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { Event, StackFrame } from '@sentry/types'; +import { Event, Mechanism, StackFrame } from '@sentry/types'; import { getGlobalObject } from './global'; import { snipLine } from './string'; @@ -125,29 +125,25 @@ export function addExceptionTypeValue(event: Event, value?: string, type?: strin } /** - * Adds exception mechanism to a given event. + * Adds exception mechanism data to a given event. Uses defaults if the second parameter is not passed. + * * @param event The event to modify. - * @param mechanism Mechanism of the mechanism. + * @param newMechanism Mechanism data to add to the event. * @hidden */ -export function addExceptionMechanism( - event: Event, - mechanism: { - [key: string]: any; - } = {}, -): void { - // TODO: Use real type with `keyof Mechanism` thingy and maybe make it better? - try { - // @ts-ignore Type 'Mechanism | {}' is not assignable to type 'Mechanism | undefined' - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - event.exception!.values![0].mechanism = event.exception!.values![0].mechanism || {}; - Object.keys(mechanism).forEach(key => { - // @ts-ignore Mechanism has no index signature - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - event.exception!.values![0].mechanism[key] = mechanism[key]; - }); - } catch (_oO) { - // no-empty +export function addExceptionMechanism(event: Event, newMechanism?: Partial): void { + if (!event.exception || !event.exception.values) { + return; + } + const exceptionValue0 = event.exception.values[0]; + + const defaultMechanism = { type: 'generic', handled: true }; + const currentMechanism = exceptionValue0.mechanism; + exceptionValue0.mechanism = { ...defaultMechanism, ...currentMechanism, ...newMechanism }; + + if (newMechanism && 'data' in newMechanism) { + const mergedData = { ...currentMechanism?.data, ...newMechanism.data }; + exceptionValue0.mechanism.data = mergedData; } } From ea2e8222dcbd3d017a801a3f2dfea86c30c3ed5e Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 19 Oct 2021 23:36:44 -0700 Subject: [PATCH 3/7] add addExceptionMechanism tests --- packages/utils/test/misc.test.ts | 39 +++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/utils/test/misc.test.ts b/packages/utils/test/misc.test.ts index c4b18e107997..440e14ddb1fd 100644 --- a/packages/utils/test/misc.test.ts +++ b/packages/utils/test/misc.test.ts @@ -228,6 +228,8 @@ describe('stripQueryStringAndFragment', () => { }); describe('addExceptionMechanism', () => { + const defaultMechanism = { type: 'generic', handled: true }; + type EventWithException = Event & { exception: { values: [{ type?: string; value?: string; mechanism?: Mechanism }]; @@ -238,7 +240,26 @@ describe('addExceptionMechanism', () => { exception: { values: [{ type: 'Error', value: 'Oh, no! Charlie ate the flip-flops! :-(' }] }, }; - it('adds data to event, preferring incoming values to current values', () => { + it('uses default values', () => { + const event = { ...baseEvent }; + + addExceptionMechanism(event); + + expect(event.exception.values[0].mechanism).toEqual(defaultMechanism); + }); + + it('prefers current values to defaults', () => { + const event = { ...baseEvent }; + + const nonDefaultMechanism = { type: 'instrument', handled: false }; + event.exception.values[0].mechanism = nonDefaultMechanism; + + addExceptionMechanism(event); + + expect(event.exception.values[0].mechanism).toEqual(nonDefaultMechanism); + }); + + it('prefers incoming values to current values', () => { const event = { ...baseEvent }; const currentMechanism = { type: 'instrument', handled: false }; @@ -250,4 +271,20 @@ describe('addExceptionMechanism', () => { // the new `handled` value took precedence expect(event.exception.values[0].mechanism).toEqual({ type: 'instrument', handled: true, synthetic: true }); }); + + it('merges data values', () => { + const event = { ...baseEvent }; + + const currentMechanism = { ...defaultMechanism, data: { function: 'addEventListener' } }; + const newMechanism = { data: { handler: 'organizeShoes', target: 'closet' } }; + event.exception.values[0].mechanism = currentMechanism; + + addExceptionMechanism(event, newMechanism); + + expect(event.exception.values[0].mechanism.data).toEqual({ + function: 'addEventListener', + handler: 'organizeShoes', + target: 'closet', + }); + }); }); From 01187b4b4b49ff6508cd944ee293dcd9bf46ee63 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 14 Oct 2021 18:49:12 -0700 Subject: [PATCH 4/7] s/handler/origHandler --- packages/nextjs/src/utils/withSentry.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index 5d2ef39d4869..5738a992fdbf 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -13,7 +13,7 @@ type WrappedNextApiHandler = NextApiHandler; type AugmentedResponse = NextApiResponse & { __sentryTransaction?: Transaction }; // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { +export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler => { // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types return async (req, res) => { // first order of business: monkeypatch `res.end()` so that it will wait for us to send events to sentry before it @@ -74,7 +74,7 @@ export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => { } try { - return await handler(req, res); // Call original handler + return await origHandler(req, res); } catch (e) { if (currentScope) { currentScope.addEventProcessor(event => { From ebde5d4485d3a8445963dce87487c3939d6eaf4e Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 14 Oct 2021 18:54:26 -0700 Subject: [PATCH 5/7] fix withSentry mechanism --- packages/nextjs/src/utils/withSentry.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index 5738a992fdbf..23b4fab2d65f 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -79,8 +79,12 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = if (currentScope) { currentScope.addEventProcessor(event => { addExceptionMechanism(event, { - mechanism: 'withSentry', - handled: false, + type: 'instrument', + handled: true, + data: { + wrapped_handler: origHandler.name, + function: 'withSentry', + }, }); return event; }); From 0f7d6a047822a58d3ff0d978826862ed006d7ecf Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 18 Oct 2021 14:08:32 -0700 Subject: [PATCH 6/7] fix serverless tests --- packages/serverless/test/awslambda.test.ts | 1 + packages/serverless/test/gcpfunction.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/serverless/test/awslambda.test.ts b/packages/serverless/test/awslambda.test.ts index f825924371c9..681582f6d296 100644 --- a/packages/serverless/test/awslambda.test.ts +++ b/packages/serverless/test/awslambda.test.ts @@ -390,6 +390,7 @@ describe('AWSLambda', () => { { mechanism: { handled: false, + type: 'generic', }, }, ], diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts index 4b5a00199796..0a77742a33c6 100644 --- a/packages/serverless/test/gcpfunction.test.ts +++ b/packages/serverless/test/gcpfunction.test.ts @@ -458,6 +458,7 @@ describe('GCPFunction', () => { { mechanism: { handled: false, + type: 'generic', }, }, ], From 707eb5daaedb660a0b4ffab91345141b31a2d9a7 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 18 Oct 2021 15:16:05 -0700 Subject: [PATCH 7/7] let event builder use defaults --- packages/browser/src/eventbuilder.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index 61dc1826423c..c3734c9c39e6 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -23,10 +23,7 @@ export function eventFromException(options: Options, exception: unknown, hint?: const event = eventFromUnknownInput(exception, syntheticException, { attachStacktrace: options.attachStacktrace, }); - addExceptionMechanism(event, { - handled: true, - type: 'generic', - }); + addExceptionMechanism(event); // defaults to { type: 'generic', handled: true } event.level = Severity.Error; if (hint && hint.event_id) { event.event_id = hint.event_id;