fix(core): Be stricter about mechanism values (#4068)

Though we have an existing `Mechanism` type, we haven't actually been using it in `addExceptionMechanism`. As a result, I didn't catch the fact that the `mechanism` data I added in #4046 was malformed, and therefore being ignored by Sentry.

This fixes both of those problems (the not using of the type and the malformed data being sent). Using the correct type also allowed `addExceptionMechanism` to be streamlined quite a bit.

Finally, given that the question of what the different attributes and their values actually mean has come up more than once, I conferred with the team, and added a docstring to the type. Hopefully this will help anyone who needs to add `mechanism` data in the future.

Author: lobsterkatie

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