fix: #542 wrong results of splitting log stacks (#547)

fix bug of stack parcer and increase compatibility for different browser vendors

src/record/error-stack-parser.ts

@@ -0,0 +1,254 @@
+/**
+ * Class StackFrame is a fork of https://github.com/stacktracejs/stackframe/blob/master/stackframe.js
+ * I fork it because:
+ * 1. There are some build issues when importing this package.
+ * 2. Rewrites into typescript give us a better type interface.
+ * 3. StackFrame contains some functions we don't need.
+ */
+export class StackFrame {
+  private fileName: string;
+  private functionName: string;
+  private lineNumber?: number;
+  private columnNumber?: number;
+
+  constructor(obj: {
+    fileName?: string;
+    functionName?: string;
+    lineNumber?: number;
+    columnNumber?: number;
+  }) {
+    this.fileName = obj.fileName || '';
+    this.functionName = obj.functionName || '';
+    this.lineNumber = obj.lineNumber;
+    this.columnNumber = obj.columnNumber;
+  }
+
+  toString() {
+    const lineNumber = this.lineNumber || '';
+    const columnNumber = this.columnNumber || '';
+    if (this.functionName) {
+      return (
+        this.functionName +
+        ' (' +
+        this.fileName +
+        ':' +
+        lineNumber +
+        ':' +
+        columnNumber +
+        ')'
+      );
+    }
+    return this.fileName + ':' + lineNumber + ':' + columnNumber;
+  }
+}
+
+/**
+ * ErrorStackParser is a fork of https://github.com/stacktracejs/error-stack-parser/blob/master/error-stack-parser.js
+ * I fork it because:
+ * 1. There are some build issues when importing this package.
+ * 2. Rewrites into typescript give us a better type interface.
+ */
+const FIREFOX_SAFARI_STACK_REGEXP = /(^|@)\S+:\d+/;
+const CHROME_IE_STACK_REGEXP = /^\s*at .*(\S+:\d+|\(native\))/m;
+const SAFARI_NATIVE_CODE_REGEXP = /^(eval@)?(\[native code])?$/;
+export const ErrorStackParser = {
+  /**
+   * Given an Error object, extract the most information from it.
+   *
+   * @param {Error} error object
+   * @return {Array} of StackFrames
+   */
+  parse: function (error: Error): StackFrame[] {
+    if (
+      // @ts-ignore
+      typeof error.stacktrace !== 'undefined' ||
+      // @ts-ignore
+      typeof error['opera#sourceloc'] !== 'undefined'
+    ) {
+      return this.parseOpera(
+        error as {
+          stacktrace?: string;
+          message: string;
+          stack?: string;
+        },
+      );
+    } else if (error.stack && error.stack.match(CHROME_IE_STACK_REGEXP)) {
+      return this.parseV8OrIE(error as { stack: string });
+    } else if (error.stack) {
+      return this.parseFFOrSafari(error as { stack: string });
+    } else {
+      throw new Error('Cannot parse given Error object');
+    }
+  },
+  // Separate line and column numbers from a string of the form: (URI:Line:Column)
+  extractLocation: function (urlLike: string) {
+    // Fail-fast but return locations like "(native)"
+    if (urlLike.indexOf(':') === -1) {
+      return [urlLike];
+    }
+
+    const regExp = /(.+?)(?::(\d+))?(?::(\d+))?$/;
+    const parts = regExp.exec(urlLike.replace(/[()]/g, ''));
+    if (!parts) throw new Error(`Cannot parse given url: ${urlLike}`);
+    return [parts[1], parts[2] || undefined, parts[3] || undefined];
+  },
+  parseV8OrIE: function (error: { stack: string }) {
+    const filtered = error.stack.split('\n').filter(function (line) {
+      return !!line.match(CHROME_IE_STACK_REGEXP);
+    }, this);
+
+    return filtered.map(function (line) {
+      if (line.indexOf('(eval ') > -1) {
+        // Throw away eval information until we implement stacktrace.js/stackframe#8
+        line = line
+          .replace(/eval code/g, 'eval')
+          .replace(/(\(eval at [^()]*)|(\),.*$)/g, '');
+      }
+      let sanitizedLine = line.replace(/^\s+/, '').replace(/\(eval code/g, '(');
+
+      // capture and preseve the parenthesized location "(/foo/my bar.js:12:87)" in
+      // case it has spaces in it, as the string is split on \s+ later on
+      const location = sanitizedLine.match(/ (\((.+):(\d+):(\d+)\)$)/);
+
+      // remove the parenthesized location from the line, if it was matched
+      sanitizedLine = location
+        ? sanitizedLine.replace(location[0], '')
+        : sanitizedLine;
+
+      const tokens = sanitizedLine.split(/\s+/).slice(1);
+      // if a location was matched, pass it to extractLocation() otherwise pop the last token
+      const locationParts = this.extractLocation(
+        location ? location[1] : tokens.pop(),
+      );
+      const functionName = tokens.join(' ') || undefined;
+      const fileName =
+        ['eval', '<anonymous>'].indexOf(locationParts[0]) > -1
+          ? undefined
+          : locationParts[0];
+
+      return new StackFrame({
+        functionName,
+        fileName,
+        lineNumber: locationParts[1],
+        columnNumber: locationParts[2],
+      });
+    }, this);
+  },
+  parseFFOrSafari: function (error: { stack: string }) {
+    const filtered = error.stack.split('\n').filter(function (line) {
+      return !line.match(SAFARI_NATIVE_CODE_REGEXP);
+    }, this);
+
+    return filtered.map(function (line) {
+      // Throw away eval information until we implement stacktrace.js/stackframe#8
+      if (line.indexOf(' > eval') > -1) {
+        line = line.replace(
+          / line (\d+)(?: > eval line \d+)* > eval:\d+:\d+/g,
+          ':$1',
+        );
+      }
+
+      if (line.indexOf('@') === -1 && line.indexOf(':') === -1) {
+        // Safari eval frames only have function names and nothing else
+        return new StackFrame({
+          functionName: line,
+        });
+      } else {
+        const functionNameRegex = /((.*".+"[^@]*)?[^@]*)(?:@)/;
+        const matches = line.match(functionNameRegex);
+        const functionName = matches && matches[1] ? matches[1] : undefined;
+        const locationParts = this.extractLocation(
+          line.replace(functionNameRegex, ''),
+        );
+
+        return new StackFrame({
+          functionName,
+          fileName: locationParts[0],
+          lineNumber: locationParts[1],
+          columnNumber: locationParts[2],
+        });
+      }
+    }, this);
+  },
+  parseOpera: function (e: {
+    stacktrace?: string;
+    message: string;
+    stack?: string;
+  }): StackFrame[] {
+    if (
+      !e.stacktrace ||
+      (e.message.indexOf('\n') > -1 &&
+        e.message.split('\n').length > e.stacktrace.split('\n').length)
+    ) {
+      return this.parseOpera9(e as { message: string });
+    } else if (!e.stack) {
+      return this.parseOpera10(e as { stacktrace: string });
+    } else {
+      return this.parseOpera11(e as { stack: string });
+    }
+  },
+  parseOpera9: function (e: { message: string }) {
+    const lineRE = /Line (\d+).*script (?:in )?(\S+)/i;
+    const lines = e.message.split('\n');
+    const result = [];
+
+    for (let i = 2, len = lines.length; i < len; i += 2) {
+      const match = lineRE.exec(lines[i]);
+      if (match) {
+        result.push(
+          new StackFrame({
+            fileName: match[2],
+            lineNumber: parseFloat(match[1]),
+          }),
+        );
+      }
+    }
+
+    return result;
+  },
+  parseOpera10: function (e: { stacktrace: string }) {
+    const lineRE = /Line (\d+).*script (?:in )?(\S+)(?:: In function (\S+))?$/i;
+    const lines = e.stacktrace.split('\n');
+    const result = [];
+
+    for (let i = 0, len = lines.length; i < len; i += 2) {
+      const match = lineRE.exec(lines[i]);
+      if (match) {
+        result.push(
+          new StackFrame({
+            functionName: match[3] || undefined,
+            fileName: match[2],
+            lineNumber: parseFloat(match[1]),
+          }),
+        );
+      }
+    }
+
+    return result;
+  },
+  // Opera 10.65+ Error.stack very similar to FF/Safari
+  parseOpera11: function (error: { stack: string }) {
+    const filtered = error.stack.split('\n').filter(function (line) {
+      return (
+        !!line.match(FIREFOX_SAFARI_STACK_REGEXP) &&
+        !line.match(/^Error created at/)
+      );
+    }, this);
+
+    return filtered.map(function (line: string) {
+      const tokens = line.split('@');
+      const locationParts = this.extractLocation(tokens.pop());
+      const functionCall = tokens.shift() || '';
+      const functionName =
+        functionCall
+          .replace(/<anonymous function(: (\w+))?>/, '$2')
+          .replace(/\([^)]*\)/g, '') || undefined;
+      return new StackFrame({
+        functionName,
+        fileName: locationParts[0],
+        lineNumber: locationParts[1],
+        columnNumber: locationParts[2],
+      });
+    }, this);
+  },
+};

src/record/observer.ts

@@ -47,6 +47,7 @@ import MutationBuffer from './mutation';
 import { stringify } from './stringify';
 import { IframeManager } from './iframe-manager';
 import { ShadowDomManager } from './shadow-dom-manager';
+import { StackFrame, ErrorStackParser } from './error-stack-parser';
 
 type WindowWithStoredMutationObserver = Window & {
   __rrMutationObserver?: MutationObserver;
@@ -602,20 +603,23 @@ function initLogObserver(
   if (logOptions.level!.includes('error')) {
     if (window) {
       const originalOnError = window.onerror;
-      // tslint:disable-next-line:no-any
+      window.onerror = (
-      window.onerror = (...args: any[]) => {
+        msg: Event | string,
+        file: string,
+        line: number,
+        col: number,
+        error: Error,
+      ) => {
         if (originalOnError) {
-          originalOnError.apply(this, args);
+          originalOnError.apply(this, [msg, file, line, col, error]);
         }
-        let stack: string[] = [];
+        const trace: string[] = ErrorStackParser.parse(
-        if (args[args.length - 1] instanceof Error) {
+          error,
-          // 0(the second parameter) tells parseStack that every stack in Error is useful
+        ).map((stackFrame: StackFrame) => stackFrame.toString());
-          stack = parseStack(args[args.length - 1].stack, 0);
+        const payload = [stringify(msg, logOptions.stringifyOptions)];
-        }
-        const payload = [stringify(args[0], logOptions.stringifyOptions)];
         cb({
           level: 'error',
-          trace: stack,
+          trace,
           payload,
         });
       };
@@ -642,11 +646,12 @@ function initLogObserver(
     }
     // replace the logger.{level}. return a restore function
     return patch(_logger, level, (original) => {
-      // tslint:disable-next-line:no-any
+      return (...args: unknown[]) => {
-      return (...args: any[]) => {
         original.apply(this, args);
         try {
-          const stack = parseStack(new Error().stack);
+          const trace = ErrorStackParser.parse(new Error())
+            .map((stackFrame: StackFrame) => stackFrame.toString())
+            .splice(1); // splice(1) to omit the hijacked log function
           const payload = args.map((s) =>
             stringify(s, logOptions.stringifyOptions),
           );
@@ -654,7 +659,7 @@ function initLogObserver(
           if (logCount < logOptions.lengthThreshold!) {
             cb({
               level,
-              trace: stack,
+              trace,
               payload,
             });
           } else if (logCount === logOptions.lengthThreshold) {
@@ -673,24 +678,6 @@ function initLogObserver(
       };
     });
   }
-  /**
-   * parse single stack message to an stack array.
-   * @param stack the stack message to be parsed
-   * @param omitDepth omit specific depth of useless stack. omit hijacked log function by default
-   */
-  function parseStack(
-    stack: string | undefined,
-    omitDepth: number = 1,
-  ): string[] {
-    let stacks: string[] = [];
-    if (stack) {
-      stacks = stack
-        .split('at')
-        .splice(1 + omitDepth)
-        .map((s) => s.trim());
-    }
-    return stacks;
-  }
 }
 
 function mergeHooks(o: observerParam, hooks: hooksParam) {

test/__snapshots__/integration.test.ts.snap

@@ -3199,8 +3199,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"assert\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:2:37\\"
-        \\"ion_script__:2:37\\"
       ],
       \\"payload\\": [
         \\"true\\",
@@ -3214,8 +3213,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"count\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:3:37\\"
-        \\"ion_script__:3:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"count\\\\\\"\\"
@@ -3228,8 +3226,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"countReset\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:4:37\\"
-        \\"ion_script__:4:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"count\\\\\\"\\"
@@ -3242,8 +3239,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"debug\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:5:37\\"
-        \\"ion_script__:5:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"debug\\\\\\"\\"
@@ -3256,8 +3252,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"dir\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:6:37\\"
-        \\"ion_script__:6:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"dir\\\\\\"\\"
@@ -3270,8 +3265,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"dirxml\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:7:37\\"
-        \\"ion_script__:7:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"dirxml\\\\\\"\\"
@@ -3284,8 +3278,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"group\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:8:37\\"
-        \\"ion_script__:8:37\\"
       ],
       \\"payload\\": []
     }
@@ -3296,8 +3289,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"groupCollapsed\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:9:37\\"
-        \\"ion_script__:9:37\\"
       ],
       \\"payload\\": []
     }
@@ -3308,8 +3300,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"info\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:10:37\\"
-        \\"ion_script__:10:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"info\\\\\\"\\"
@@ -3322,8 +3313,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"log\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:11:37\\"
-        \\"ion_script__:11:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"log\\\\\\"\\"
@@ -3336,8 +3326,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"table\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:12:37\\"
-        \\"ion_script__:12:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"table\\\\\\"\\"
@@ -3350,8 +3339,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"time\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:13:37\\"
-        \\"ion_script__:13:37\\"
       ],
       \\"payload\\": []
     }
@@ -3362,8 +3350,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"timeEnd\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:14:37\\"
-        \\"ion_script__:14:37\\"
       ],
       \\"payload\\": []
     }
@@ -3374,8 +3361,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"timeLog\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:15:37\\"
-        \\"ion_script__:15:37\\"
       ],
       \\"payload\\": []
     }
@@ -3386,8 +3372,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"trace\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:16:37\\"
-        \\"ion_script__:16:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"trace\\\\\\"\\"
@@ -3400,8 +3385,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"warn\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:17:37\\"
-        \\"ion_script__:17:37\\"
       ],
       \\"payload\\": [
         \\"\\\\\\"warn\\\\\\"\\"
@@ -3414,8 +3398,7 @@ exports[`log 1`] = `
       \\"source\\": 11,
       \\"level\\": \\"clear\\",
       \\"trace\\": [
-        \\"__puppeteer_evalu\\",
+        \\"__puppeteer_evaluation_script__:18:37\\"
-        \\"ion_script__:18:37\\"
       ],
       \\"payload\\": []
     }

test/integration.test.ts

@@ -379,7 +379,7 @@ describe('record integration tests', function (this: ISuite) {
     expect(text).to.equal('4\n3\n2\n1\n5');
   });
 
-  it('can record log mutation', async () => {
+  it('should record console messages', async () => {
     const page: puppeteer.Page = await this.browser.newPage();
     await page.goto('about:blank');
     await page.setContent(

typings/record/error-stack-parser.d.ts

@@ -0,0 +1,37 @@
+export declare class StackFrame {
+    private fileName;
+    private functionName;
+    private lineNumber?;
+    private columnNumber?;
+    constructor(obj: {
+        fileName?: string;
+        functionName?: string;
+        lineNumber?: number;
+        columnNumber?: number;
+    });
+    toString(): string;
+}
+export declare const ErrorStackParser: {
+    parse: (error: Error) => StackFrame[];
+    extractLocation: (urlLike: string) => (string | undefined)[];
+    parseV8OrIE: (error: {
+        stack: string;
+    }) => StackFrame[];
+    parseFFOrSafari: (error: {
+        stack: string;
+    }) => StackFrame[];
+    parseOpera: (e: {
+        stacktrace?: string;
+        message: string;
+        stack?: string;
+    }) => StackFrame[];
+    parseOpera9: (e: {
+        message: string;
+    }) => StackFrame[];
+    parseOpera10: (e: {
+        stacktrace: string;
+    }) => StackFrame[];
+    parseOpera11: (error: {
+        stack: string;
+    }) => StackFrame[];
+};