ref(browser): Split stack line parsers into individual functions and simplify further (#4555)

- Converts parsers into individual functions now in `stack-parsers.ts`
- Removes `TraceKitStackTrace` and instead just returns `StackFrame[]`
- Replaces `exceptionFromStacktrace(computeStackTrace(e))` with `exceptionFromError(e))` 
- Replaces `eventFromStacktrace(computeStackTrace(e))` with `eventFromError(e))` 
- Changes tests to test against a full `Exception` which meant frames needed reversing and `in_app` adding
- Moves `createStackParser` and `StackLineParser` type to `@sentry/utils` where there was already a `stacktrace.ts`
- Renamed `prepareFramesForEvent` to `stripSentryFramesAndReverse` and moved to utils too. This is nearly identical to the node.js code and means the parser now returns frames ready to go into `Exception`
- Moved `in_app` to `createFrame` in stack parsers since everything browser is considered `in_app`
- Simplified frame popping so it occurs before parsing. ie. `stack.split('\n').slice(skipFirst)`

Author: timfish

packages/browser/src/eventbuilder.ts

@@ -11,8 +11,7 @@ import {
   resolvedSyncPromise,
 } from '@sentry/utils';
 
-import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
-import { computeStackTrace } from './tracekit';
+import { eventFromError, eventFromPlainObject, parseStackFrames } from './parsers';
 
 /**
  * Creates an {@link Event} from all inputs to `captureException` and non-primitive inputs to `captureMessage`.
@@ -68,10 +67,7 @@ export function eventFromUnknownInput(
   if (isErrorEvent(exception as ErrorEvent) && (exception as ErrorEvent).error) {
     // If it is an ErrorEvent with `error` property, extract it to get actual Error
     const errorEvent = exception as ErrorEvent;
-    // eslint-disable-next-line no-param-reassign
-    exception = errorEvent.error;
-    event = eventFromStacktrace(computeStackTrace(exception as Error));
-    return event;
+    return eventFromError(errorEvent.error as Error);
   }
 
   // If it is a `DOMError` (which is a legacy API, but still supported in some browsers) then we just extract the name
@@ -85,7 +81,7 @@ export function eventFromUnknownInput(
     const domException = exception as DOMException;
 
     if ('stack' in (exception as Error)) {
-      event = eventFromStacktrace(computeStackTrace(exception as Error));
+      event = eventFromError(exception as Error);
     } else {
       const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException');
       const message = domException.message ? `${name}: ${domException.message}` : name;
@@ -100,8 +96,7 @@ export function eventFromUnknownInput(
   }
   if (isError(exception as Error)) {
     // we have a real Error object, do nothing
-    event = eventFromStacktrace(computeStackTrace(exception as Error));
-    return event;
+    return eventFromError(exception as Error);
   }
   if (isPlainObject(exception) || isEvent(exception)) {
     // If it's a plain object or an instance of `Event` (the built-in JS kind, not this SDK's `Event` type), serialize
@@ -148,10 +143,8 @@ export function eventFromString(
   };
 
   if (options.attachStacktrace && syntheticException) {
-    const stacktrace = computeStackTrace(syntheticException);
-    const frames = prepareFramesForEvent(stacktrace.stack);
     event.stacktrace = {
-      frames,
+      frames: parseStackFrames(syntheticException),
     };
   }
 

packages/browser/src/integrations/linkederrors.ts

@@ -2,8 +2,7 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core';
 import { Event, EventHint, Exception, ExtendedError, Integration } from '@sentry/types';
 import { isInstanceOf } from '@sentry/utils';
 
-import { exceptionFromStacktrace } from '../parsers';
-import { computeStackTrace } from '../tracekit';
+import { exceptionFromError } from '../parsers';
 
 const DEFAULT_KEY = 'cause';
 const DEFAULT_LIMIT = 5;
@@ -73,7 +72,6 @@ export function _walkErrorTree(limit: number, error: ExtendedError, key: string,
   if (!isInstanceOf(error[key], Error) || stack.length + 1 >= limit) {
     return stack;
   }
-  const stacktrace = computeStackTrace(error[key]);
-  const exception = exceptionFromStacktrace(stacktrace);
+  const exception = exceptionFromError(error[key]);
   return _walkErrorTree(limit, error[key], key, [exception, ...stack]);
 }

packages/browser/src/parsers.ts

@@ -1,21 +1,21 @@
+/* eslint-disable @typescript-eslint/no-unsafe-member-access */
 import { Event, Exception, StackFrame } from '@sentry/types';
-import { extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils';
+import { createStackParser, extractExceptionKeysForMessage, isEvent, normalizeToSize } from '@sentry/utils';
 
-import { computeStackTrace, StackTrace as TraceKitStackTrace } from './tracekit';
-
-const STACKTRACE_LIMIT = 50;
+import { chrome, gecko, opera10, opera11, winjs } from './stack-parsers';
 
 /**
  * This function creates an exception from an TraceKitStackTrace
  * @param stacktrace TraceKitStackTrace that will be converted to an exception
  * @hidden
  */
-export function exceptionFromStacktrace(stacktrace: TraceKitStackTrace): Exception {
-  const frames = prepareFramesForEvent(stacktrace.stack);
+export function exceptionFromError(ex: Error): Exception {
+  // Get the frames first since Opera can lose the stack if we touch anything else first
+  const frames = parseStackFrames(ex);
 
   const exception: Exception = {
-    type: stacktrace.name,
-    value: stacktrace.message,
+    type: ex && ex.name,
+    value: extractMessage(ex),
   };
 
   if (frames && frames.length) {
@@ -54,10 +54,8 @@ export function eventFromPlainObject(
   };
 
   if (syntheticException) {
-    const stacktrace = computeStackTrace(syntheticException);
-    const frames = prepareFramesForEvent(stacktrace.stack);
     event.stacktrace = {
-      frames,
+      frames: parseStackFrames(syntheticException),
     };
   }
 
@@ -67,48 +65,63 @@ export function eventFromPlainObject(
 /**
  * @hidden
  */
-export function eventFromStacktrace(stacktrace: TraceKitStackTrace): Event {
-  const exception = exceptionFromStacktrace(stacktrace);
-
+export function eventFromError(ex: Error): Event {
   return {
     exception: {
-      values: [exception],
+      values: [exceptionFromError(ex)],
     },
   };
 }
 
-/**
- * @hidden
- */
-export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] {
-  if (!stack.length) {
-    return [];
+/** Parses stack frames from an error */
+export function parseStackFrames(ex: Error & { framesToPop?: number; stacktrace?: string }): StackFrame[] {
+  // Access and store the stacktrace property before doing ANYTHING
+  // else to it because Opera is not very good at providing it
+  // reliably in other circumstances.
+  const stacktrace = ex.stacktrace || ex.stack || '';
+
+  const popSize = getPopSize(ex);
+
+  try {
+    // The order of the parsers in important
+    return createStackParser(opera10, opera11, chrome, winjs, gecko)(stacktrace, popSize);
+  } catch (e) {
+    // no-empty
   }
 
-  let localStack = stack;
+  return [];
+}
 
-  const firstFrameFunction = localStack[0].function || '';
-  const lastFrameFunction = localStack[localStack.length - 1].function || '';
+// Based on our own mapping pattern - https://github.com/getsentry/sentry/blob/9f08305e09866c8bd6d0c24f5b0aabdd7dd6c59c/src/sentry/lang/javascript/errormapping.py#L83-L108
+const reactMinifiedRegexp = /Minified React error #\d+;/i;
 
-  // If stack starts with one of our API calls, remove it (starts, meaning it's the top of the stack - aka last call)
-  if (firstFrameFunction.indexOf('captureMessage') !== -1 || firstFrameFunction.indexOf('captureException') !== -1) {
-    localStack = localStack.slice(1);
-  }
+function getPopSize(ex: Error & { framesToPop?: number }): number {
+  if (ex) {
+    if (typeof ex.framesToPop === 'number') {
+      return ex.framesToPop;
+    }
 
-  // If stack ends with one of our internal API calls, remove it (ends, meaning it's the bottom of the stack - aka top-most call)
-  if (lastFrameFunction.indexOf('sentryWrapped') !== -1) {
-    localStack = localStack.slice(0, -1);
+    if (reactMinifiedRegexp.test(ex.message)) {
+      return 1;
+    }
   }
 
-  // The frame where the crash happened, should be the last entry in the array
-  return localStack
-    .slice(0, STACKTRACE_LIMIT)
-    .map(frame => ({
-      filename: frame.filename || localStack[0].filename,
-      function: frame.function || '?',
-      lineno: frame.lineno,
-      colno: frame.colno,
-      in_app: true,
-    }))
-    .reverse();
+  return 0;
+}
+
+/**
+ * There are cases where stacktrace.message is an Event object
+ * https://github.com/getsentry/sentry-javascript/issues/1949
+ * In this specific case we try to extract stacktrace.message.error.message
+ */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+function extractMessage(ex: any): string {
+  const message = ex && ex.message;
+  if (!message) {
+    return 'No error message';
+  }
+  if (message.error && typeof message.error.message === 'string') {
+    return message.error.message;
+  }
+  return message;
 }

packages/browser/src/stack-parsers.ts

@@ -0,0 +1,148 @@
+import { StackFrame } from '@sentry/types';
+import { StackLineParser } from '@sentry/utils';
+
+// global reference to slice
+const UNKNOWN_FUNCTION = '?';
+
+function createFrame(filename: string, func: string, lineno?: number, colno?: number): StackFrame {
+  const frame: StackFrame = {
+    filename,
+    function: func,
+    // All browser frames are considered in_app
+    in_app: true,
+  };
+
+  if (lineno !== undefined) {
+    frame.lineno = lineno;
+  }
+
+  if (colno !== undefined) {
+    frame.colno = colno;
+  }
+
+  return frame;
+}
+
+// Chromium based browsers: Chrome, Brave, new Opera, new Edge
+const chromeRegex =
+  /^\s*at (?:(.*?) ?\((?:address at )?)?((?:file|https?|blob|chrome-extension|address|native|eval|webpack|<anonymous>|[-a-z]+:|.*bundle|\/).*?)(?::(\d+))?(?::(\d+))?\)?\s*$/i;
+const chromeEvalRegex = /\((\S*)(?::(\d+))(?::(\d+))\)/;
+
+export const chrome: StackLineParser = line => {
+  const parts = chromeRegex.exec(line);
+
+  if (parts) {
+    const isEval = parts[2] && parts[2].indexOf('eval') === 0; // start of line
+
+    if (isEval) {
+      const subMatch = chromeEvalRegex.exec(parts[2]);
+
+      if (subMatch) {
+        // throw out eval line/column and use top-most line/column number
+        parts[2] = subMatch[1]; // url
+        parts[3] = subMatch[2]; // line
+        parts[4] = subMatch[3]; // column
+      }
+    }
+
+    // Kamil: One more hack won't hurt us right? Understanding and adding more rules on top of these regexps right now
+    // would be way too time consuming. (TODO: Rewrite whole RegExp to be more readable)
+    const [func, filename] = extractSafariExtensionDetails(parts[1] || UNKNOWN_FUNCTION, parts[2]);
+
+    return createFrame(filename, func, parts[3] ? +parts[3] : undefined, parts[4] ? +parts[4] : undefined);
+  }
+
+  return;
+};
+
+// gecko regex: `(?:bundle|\d+\.js)`: `bundle` is for react native, `\d+\.js` also but specifically for ram bundles because it
+// generates filenames without a prefix like `file://` the filenames in the stacktrace are just 42.js
+// We need this specific case for now because we want no other regex to match.
+const geckoREgex =
+  /^\s*(.*?)(?:\((.*?)\))?(?:^|@)?((?:file|https?|blob|chrome|webpack|resource|moz-extension|capacitor).*?:\/.*?|\[native code\]|[^@]*(?:bundle|\d+\.js)|\/[\w\-. /=]+)(?::(\d+))?(?::(\d+))?\s*$/i;
+const geckoEvalRegex = /(\S+) line (\d+)(?: > eval line \d+)* > eval/i;
+
+export const gecko: StackLineParser = line => {
+  const parts = geckoREgex.exec(line);
+
+  if (parts) {
+    const isEval = parts[3] && parts[3].indexOf(' > eval') > -1;
+    if (isEval) {
+      const subMatch = geckoEvalRegex.exec(parts[3]);
+
+      if (subMatch) {
+        // throw out eval line/column and use top-most line number
+        parts[1] = parts[1] || `eval`;
+        parts[3] = subMatch[1];
+        parts[4] = subMatch[2];
+        parts[5] = ''; // no column when eval
+      }
+    }
+
+    let filename = parts[3];
+    let func = parts[1] || UNKNOWN_FUNCTION;
+    [func, filename] = extractSafariExtensionDetails(func, filename);
+
+    return createFrame(filename, func, parts[4] ? +parts[4] : undefined, parts[5] ? +parts[5] : undefined);
+  }
+
+  return;
+};
+
+const winjsRegex =
+  /^\s*at (?:((?:\[object object\])?.+) )?\(?((?:file|ms-appx|https?|webpack|blob):.*?):(\d+)(?::(\d+))?\)?\s*$/i;
+
+export const winjs: StackLineParser = line => {
+  const parts = winjsRegex.exec(line);
+
+  return parts
+    ? createFrame(parts[2], parts[1] || UNKNOWN_FUNCTION, +parts[3], parts[4] ? +parts[4] : undefined)
+    : undefined;
+};
+
+const opera10Regex = / line (\d+).*script (?:in )?(\S+)(?:: in function (\S+))?$/i;
+
+export const opera10: StackLineParser = line => {
+  const parts = opera10Regex.exec(line);
+  return parts ? createFrame(parts[2], parts[3] || UNKNOWN_FUNCTION, +parts[1]) : undefined;
+};
+
+const opera11Regex =
+  / line (\d+), column (\d+)\s*(?:in (?:<anonymous function: ([^>]+)>|([^)]+))\(.*\))? in (.*):\s*$/i;
+
+export const opera11: StackLineParser = line => {
+  const parts = opera11Regex.exec(line);
+  return parts ? createFrame(parts[5], parts[3] || parts[4] || UNKNOWN_FUNCTION, +parts[1], +parts[2]) : undefined;
+};
+
+/**
+ * Safari web extensions, starting version unknown, can produce "frames-only" stacktraces.
+ * What it means, is that instead of format like:
+ *
+ * Error: wat
+ *   at function@url:row:col
+ *   at function@url:row:col
+ *   at function@url:row:col
+ *
+ * it produces something like:
+ *
+ *   function@url:row:col
+ *   function@url:row:col
+ *   function@url:row:col
+ *
+ * Because of that, it won't be captured by `chrome` RegExp and will fall into `Gecko` branch.
+ * This function is extracted so that we can use it in both places without duplicating the logic.
+ * Unfortunately "just" changing RegExp is too complicated now and making it pass all tests
+ * and fix this case seems like an impossible, or at least way too time-consuming task.
+ */
+const extractSafariExtensionDetails = (func: string, filename: string): [string, string] => {
+  const isSafariExtension = func.indexOf('safari-extension') !== -1;
+  const isSafariWebExtension = func.indexOf('safari-web-extension') !== -1;
+
+  return isSafariExtension || isSafariWebExtension
+    ? [
+        func.indexOf('@') !== -1 ? func.split('@')[0] : UNKNOWN_FUNCTION,
+        isSafariExtension ? `safari-extension:${filename}` : `safari-web-extension:${filename}`,
+      ]
+    : [func, filename];
+};