fix(core): Be stricter about mechanism values

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;

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,13 +74,17 @@ 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 => {
             addExceptionMechanism(event, {
-              mechanism: 'withSentry',
-              handled: false,
+              type: 'instrument',
+              handled: true,
+              data: {
+                wrapped_handler: origHandler.name,
+                function: 'withSentry',
+              },
             });
             return event;
           });

packages/serverless/test/awslambda.test.ts

@@ -390,6 +390,7 @@ describe('AWSLambda', () => {
             {
               mechanism: {
                 handled: false,
+                type: 'generic',
               },
             },
           ],

packages/serverless/test/gcpfunction.test.ts

@@ -458,6 +458,7 @@ describe('GCPFunction', () => {
             {
               mechanism: {
                 handled: false,
+                type: 'generic',
               },
             },
           ],

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;
 }

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<Mechanism>): 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;
   }
 }
 

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',
+    });
+  });
 });