diff --git a/packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts b/packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts index ee65f8b9de..183eb93c45 100644 --- a/packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts +++ b/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'), { diff --git a/packages/telemetry/browser-telemetry/__tests__/vendor/TraceKit/TraceKit.test.ts b/packages/telemetry/browser-telemetry/__tests__/vendor/TraceKit/TraceKit.test.ts index ddb6f15320..ac368acb68 100644 --- a/packages/telemetry/browser-telemetry/__tests__/vendor/TraceKit/TraceKit.test.ts +++ b/packages/telemetry/browser-telemetry/__tests__/vendor/TraceKit/TraceKit.test.ts @@ -114,4 +114,123 @@ describe('TraceKit', function () { expect(stackFrames.stack[0].url).toEqual(''); }); }); + + 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, + }); + }); + }); }); diff --git a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts index d4563359ca..661375fcab 100644 --- a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts +++ b/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 { diff --git a/packages/telemetry/browser-telemetry/src/vendor/TraceKit.ts b/packages/telemetry/browser-telemetry/src/vendor/TraceKit.ts index a832ddf224..57b605b95a 100644 --- a/packages/telemetry/browser-telemetry/src/vendor/TraceKit.ts +++ b/packages/telemetry/browser-telemetry/src/vendor/TraceKit.ts @@ -11,6 +11,9 @@ * Functionality unused by this SDK has been removed to minimize size. * * It has additionally been converted to typescript. + * + * The functionality of computeStackTrace has been extended to allow for easier use of the context + * information in stack frames. */ /** @@ -34,6 +37,23 @@ /* eslint-disable no-continue */ /* eslint-disable no-underscore-dangle */ +/** + * Source context information. + * + * This was not present in the original source, but helps to easily identify which line in the + * context is the original line. + * + * Without this knowledge of the source file is requires for a consumer to know the position + * of the original source line within the context. + */ +export interface SourceContext { + /** + * The starting location in the source code. This is 1-based. + */ + startFromSource?: number; + contextLines: string[] | null; +} + export interface TraceKitStatic { computeStackTrace: { (ex: Error, depth?: number): StackTrace; @@ -45,7 +65,7 @@ export interface TraceKitStatic { ) => boolean; computeStackTraceFromStackProp: (ex: Error) => StackTrace | null; guessFunctionName: (url: string, lineNo: number | string) => string; - gatherContext: (url: string, line: number | string) => string[] | null; + gatherContext: (url: string, line: number | string) => SourceContext | null; ofCaller: (depth?: number) => StackTrace; getSource: (url: string) => string[]; }; @@ -61,9 +81,18 @@ export interface StackFrame { url: string; func: string; args?: string[]; - line?: number; - column?: number; - context?: string[]; + /** + * The line number of source code. + * This is 1-based. + */ + line?: number | null; + column?: number | null; + context?: string[] | null; + /** + * The source line number that is the first line of the context. + * This is 1-based. + */ + srcStart?: number; } export type Mode = 'stack' | 'stacktrace' | 'multiline' | 'callers' | 'onerror' | 'failed'; @@ -315,10 +344,10 @@ export interface StackTrace { * Retrieves the surrounding lines from where an exception occurred. * @param {string} url URL of source code. * @param {(string|number)} line Line number in source code to center around for context. - * @return {?Array.} Lines of source code. + * @return {SourceContext} Lines of source code and line the source context starts on. * @memberof TraceKit.computeStackTrace */ - function gatherContext(url: string, line: string | number): string[] | null { + function gatherContext(url: string, line: string | number): SourceContext | null { if (typeof line !== 'number') { line = Number(line); } @@ -346,9 +375,14 @@ export interface StackTrace { } } - return context.length > 0 ? context : null; + return context.length > 0 + ? { + contextLines: context, + // Source lines are in base-1. + startFromSource: start + 1, + } + : null; } - /** * Escapes special characters, except for whitespace, in a string to be * used inside a regular expression as a string literal. @@ -573,10 +607,10 @@ export interface StackTrace { const chromeEval = /\((\S*)(?::(\d+))(?::(\d+))\)/; const lines = ex.stack.split('\n'); - const stack: any = []; + const stack: StackFrame[] = []; let submatch: any; let parts: any; - let element: any; + let element: StackFrame; const reference: any = /^(.*) is undefined$/.exec(ex.message); for (let i = 0, j = lines.length; i < j; ++i) { @@ -633,7 +667,9 @@ export interface StackTrace { element.func = guessFunctionName(element.url, element.line); } - element.context = element.line ? gatherContext(element.url, element.line) : null; + const srcContext = gatherContext(element.url, element.line!); + element.context = element.line ? (srcContext?.contextLines ?? null) : null; + element.srcStart = srcContext?.startFromSource; stack.push(element); } @@ -677,7 +713,7 @@ export interface StackTrace { let parts; for (let line = 0; line < lines.length; line += 2) { - let element: any = null; + let element: StackFrame | null = null; if ((parts = opera10Regex.exec(lines[line]))) { element = { url: parts[2], @@ -702,7 +738,9 @@ export interface StackTrace { } if (element.line) { try { - element.context = gatherContext(element.url, element.line); + const srcContext = gatherContext(element.url, element.line); + element.srcStart = srcContext?.startFromSource; + element.context = element.line ? (srcContext?.contextLines ?? null) : null; } catch (exc) {} } @@ -820,7 +858,9 @@ export interface StackTrace { if (!item.func) { item.func = guessFunctionName(item.url, item.line); } - const context = gatherContext(item.url, item.line); + const srcContext = gatherContext(item.url, item.line); + item.srcStart = srcContext?.startFromSource; + const context = srcContext?.contextLines; const midline = context ? context[Math.floor(context.length / 2)] : null; if ( context && @@ -880,7 +920,9 @@ export interface StackTrace { } if (!initial.context) { - initial.context = gatherContext(initial.url, initial.line); + const srcContext = gatherContext(initial.url, initial.line); + initial.context = srcContext?.contextLines ?? null; + initial.srcStart = srcContext?.startFromSource; } const reference = / '([^']+)' /.exec(message);