fix: Fix a bug where the incorrect src lines may have been captured.

packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts

@@ -52,6 +52,8 @@ describe('given source lines', () => {
 describe('given an input stack frame', () => {
   const inputFrame = {
     context: ['1234567890', 'ABCDEFGHIJ', 'the src line', '0987654321', 'abcdefghij'],
+    line: 3,
+    srcStart: 1,
     column: 0,
   };
 
@@ -124,6 +126,56 @@ describe('given an input stack frame', () => {
   });
 });
 
+it('can handle an origin before the context window', () => {
+  // This isn't expected, but we just want to make sure it is handle gracefully.
+  const inputFrame = {
+    context: ['1234567890', 'ABCDEFGHIJ', 'the src line', '0987654321', 'abcdefghij'],
+    line: 3,
+    srcStart: 5,
+    column: 0,
+  };
+
+  expect(
+    getSrcLines(inputFrame, {
+      enabled: true,
+      source: {
+        beforeLines: 1,
+        afterLines: 1,
+        maxLineLength: 280,
+      },
+    }),
+  ).toMatchObject({
+    srcBefore: [],
+    srcLine: '1234567890',
+    srcAfter: ['ABCDEFGHIJ'],
+  });
+});
+
+it('can handle an origin after the context window', () => {
+  // This isn't expected, but we just want to make sure it is handle gracefully.
+  const inputFrame = {
+    context: ['1234567890', 'ABCDEFGHIJ', 'the src line', '0987654321', 'abcdefghij'],
+    line: 100,
+    srcStart: 5,
+    column: 0,
+  };
+
+  expect(
+    getSrcLines(inputFrame, {
+      enabled: true,
+      source: {
+        beforeLines: 1,
+        afterLines: 1,
+        maxLineLength: 280,
+      },
+    }),
+  ).toMatchObject({
+    srcBefore: ['0987654321'],
+    srcLine: 'abcdefghij',
+    srcAfter: [],
+  });
+});
+
 it('returns an empty stack when stack parsing is disabled', () => {
   expect(
     parse(new Error('test'), {

packages/telemetry/browser-telemetry/__tests__/vendor/TraceKit/TraceKit.test.ts

@@ -114,4 +114,123 @@ describe('TraceKit', function () {
       expect(stackFrames.stack[0].url).toEqual('<anonymous>');
     });
   });
+
+  describe('given mock source code, xhr, and domain', () => {
+    // Mock source code that will be fetched
+    const mockSource =
+      'function foo() {\n' +
+      '  console.log("line 2");\n' +
+      '  throw new Error("error on line 3");\n' +
+      '  console.log("line 4");\n' +
+      '}\n' +
+      'foo();';
+
+    // Mock XMLHttpRequest
+    const mockXHR = {
+      open: jest.fn(),
+      send: jest.fn(),
+      responseText: mockSource,
+    };
+
+    // @ts-ignore - we know this is incomplete
+    window.XMLHttpRequest = jest.fn(() => mockXHR);
+
+    window.document.domain = 'localhost';
+
+    it('should populate srcStart and context from source code with firefox style stack trace', () => {
+      const traceKit = getTraceKit();
+      traceKit.remoteFetching = true;
+      traceKit.linesOfContext = 10;
+
+      const error = new Error('error on line 3');
+      // Firefox style stack trace
+      error.stack =
+        'foo/<@http://localhost:8081/assets/index-BvsURM3r.js:3:2\n' +
+        '@http://localhost:8081/assets/index-BvsURM3r.js:6:0';
+      const stackFrames = traceKit.computeStackTrace(error);
+
+      expect(stackFrames).toBeTruthy();
+      expect(stackFrames.stack[0]).toEqual({
+        url: 'http://localhost:8081/assets/index-BvsURM3r.js',
+        func: 'foo/<',
+        args: [],
+        line: 3,
+        column: 2,
+        context: [
+          'function foo() {',
+          '  console.log("line 2");',
+          '  throw new Error("error on line 3");',
+          '  console.log("line 4");',
+          '}',
+          'foo();',
+        ],
+        srcStart: 1,
+      });
+    });
+
+    it('should populate srcStart and context from source code with chrome style stack trace', () => {
+      const traceKit = getTraceKit();
+      traceKit.remoteFetching = true;
+      traceKit.linesOfContext = 10;
+
+      const error = new Error('error on line 3');
+      // Chrome style stack trace
+      error.stack =
+        'Error: error on line 3\n' +
+        '    at foo (http://localhost:8081/assets/index-BvsURM3r.js:3:2)\n' +
+        '    at http://localhost:8081/assets/index-BvsURM3r.js:6:0';
+      const stackFrames = traceKit.computeStackTrace(error);
+
+      expect(stackFrames).toBeTruthy();
+      expect(stackFrames.stack[0]).toEqual({
+        url: 'http://localhost:8081/assets/index-BvsURM3r.js',
+        func: 'foo',
+        args: [],
+        line: 3,
+        column: 2,
+        context: [
+          'function foo() {',
+          '  console.log("line 2");',
+          '  throw new Error("error on line 3");',
+          '  console.log("line 4");',
+          '}',
+          'foo();',
+        ],
+        srcStart: 1,
+      });
+    });
+
+    it('should populate srcStart and context from source code with opera style stack trace', () => {
+      const traceKit = getTraceKit();
+      traceKit.remoteFetching = true;
+      traceKit.linesOfContext = 10;
+
+      const error = new Error('error on line 3');
+      // Opera style stack trace
+      // @ts-ignore - Opera does what it wants.
+      error.stacktrace =
+        'Error initially occurred at line 3, column 2 in foo() in http://localhost:8081/assets/index-BvsURM3r.js:\n' +
+        'throw new Error("error on line 3");\n' +
+        'called from line 6, column 1 in foo() in http://localhost:8081/assets/index-BvsURM3r.js:';
+      const stackFrames = traceKit.computeStackTrace(error);
+
+      expect(stackFrames).toBeTruthy();
+      expect(stackFrames.stack[0]).toEqual({
+        url: 'http://localhost:8081/assets/index-BvsURM3r.js',
+        func: 'foo',
+        args: [],
+        line: 3,
+        column: 2,
+        context: [
+          'function foo() {',
+          '  console.log("line 2");',
+          '  throw new Error("error on line 3");',
+          '  console.log("line 4");',
+          '}',
+          'foo();',
+        ],
+        srcStart: 1,
+      });
+    });
+  });
 });

packages/telemetry/browser-telemetry/src/stack/StackParser.ts

@@ -47,6 +47,18 @@ export function processUrlToFileName(input: string, origin: string): string {
   return cleaned;
 }
 
+/**
+ * Clamp a value to be between an inclusive max an minimum.
+ *
+ * @param min The inclusive minimum value.
+ * @param max The inclusive maximum value.
+ * @param value The value to clamp.
+ * @returns The clamped value in range [min, max].
+ */
+function clamp(min: number, max: number, value: number): number {
+  return Math.min(max, Math.max(min, value));
+}
+
 export interface TrimOptions {
   /**
    * The maximum length of the trimmed line.
@@ -130,6 +142,8 @@ export function getSrcLines(
     // as we can.
     context?: string[] | null;
     column?: number | null;
+    srcStart?: number | null;
+    line?: number | null;
   },
   options: ParsedStackOptions,
 ): {
@@ -168,7 +182,8 @@ export function getSrcLines(
       0,
     );
 
-  const origin = Math.floor(context.length / 2);
+  const origin = clamp(0, context.length - 1, (inFrame?.line ?? 0) - (inFrame.srcStart ?? 0));
+
   return {
     // The lines immediately preceeding the origin line.
     srcBefore: getLines(origin - options.source.beforeLines, origin, context, trimmer),
@@ -204,8 +219,9 @@ export default function parse(error: Error, options: ParsedStackOptions): StackT
   const frames: StackFrame[] = parsed.stack.reverse().map((inFrame) => ({
     fileName: processUrlToFileName(inFrame.url, window.location.origin),
     function: inFrame.func,
-    line: inFrame.line,
-    col: inFrame.column,
+    // Strip the nulls so we only ever return undefined.
+    line: inFrame.line ?? undefined,
+    col: inFrame.column ?? undefined,
     ...getSrcLines(inFrame, options),
   }));
   return {