From 8b547c1ea9f2ec005c593ef810b890bee0615f2f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 8 Nov 2024 13:09:53 -0800 Subject: [PATCH 1/6] feat: Add stack trace parsing and options. Testing. remove stack parser --- .../__tests__/options.test.ts | 422 ++++++++++++++++++ .../browser-telemetry/src/MinLogger.ts | 9 + .../browser-telemetry/src/options.ts | 285 ++++++++++++ 3 files changed, 716 insertions(+) create mode 100644 packages/telemetry/browser-telemetry/__tests__/options.test.ts create mode 100644 packages/telemetry/browser-telemetry/src/MinLogger.ts create mode 100644 packages/telemetry/browser-telemetry/src/options.ts diff --git a/packages/telemetry/browser-telemetry/__tests__/options.test.ts b/packages/telemetry/browser-telemetry/__tests__/options.test.ts new file mode 100644 index 000000000..1d5004ba0 --- /dev/null +++ b/packages/telemetry/browser-telemetry/__tests__/options.test.ts @@ -0,0 +1,422 @@ +import ErrorCollector from '../src/collectors/error'; +import parse, { defaultOptions } from '../src/options'; + +const mockLogger = { + warn: jest.fn(), +}; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it('handles an empty configuration', () => { + const outOptions = parse({}); + expect(outOptions).toEqual(defaultOptions()); +}); + +it('can set all options at once', () => { + const outOptions = parse({ + maxPendingEvents: 1, + breadcrumbs: { + maxBreadcrumbs: 1, + click: false, + evaluations: false, + flagChange: false, + }, + collectors: [new ErrorCollector(), new ErrorCollector()], + }); + expect(outOptions).toEqual({ + maxPendingEvents: 1, + breadcrumbs: { + keyboardInput: true, + maxBreadcrumbs: 1, + click: false, + evaluations: false, + flagChange: false, + http: { + customUrlFilter: undefined, + instrumentFetch: true, + instrumentXhr: true, + }, + }, + stack: { + source: { + beforeLines: 3, + afterLines: 3, + maxLineLength: 280, + }, + }, + collectors: [new ErrorCollector(), new ErrorCollector()], + }); +}); + +it('warns when maxPendingEvents is not a number', () => { + const outOptions = parse( + { + // @ts-ignore + maxPendingEvents: 'not a number', + }, + mockLogger, + ); + + expect(outOptions.maxPendingEvents).toEqual(defaultOptions().maxPendingEvents); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "maxPendingEvents" should be of type number, got string, using default value', + ); +}); + +it('accepts valid maxPendingEvents number', () => { + const outOptions = parse( + { + maxPendingEvents: 100, + }, + mockLogger, + ); + + expect(outOptions.maxPendingEvents).toEqual(100); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('warns when breadcrumbs config is not an object', () => { + const outOptions = parse( + { + // @ts-ignore + breadcrumbs: 'not an object', + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs).toEqual(defaultOptions().breadcrumbs); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs" should be of type object, got string, using default value', + ); +}); + +it('warns when collectors is not an array', () => { + const outOptions = parse( + { + // @ts-ignore + collectors: 'not an array', + }, + mockLogger, + ); + + expect(outOptions.collectors).toEqual(defaultOptions().collectors); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "collectors" should be of type Collector[], got string, using default value', + ); +}); + +it('accepts valid collectors array', () => { + const collectors = [new ErrorCollector()]; + const outOptions = parse( + { + collectors, + }, + mockLogger, + ); + + expect(outOptions.collectors).toEqual(collectors); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('warns when stack config is not an object', () => { + const outOptions = parse( + { + // @ts-ignore + stack: 'not an object', + }, + mockLogger, + ); + + expect(outOptions.stack).toEqual(defaultOptions().stack); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "stack" should be of type object, got string, using default value', + ); +}); + +it('warns when breadcrumbs.maxBreadcrumbs is not a number', () => { + const outOptions = parse( + { + breadcrumbs: { + // @ts-ignore + maxBreadcrumbs: 'not a number', + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.maxBreadcrumbs).toEqual( + defaultOptions().breadcrumbs.maxBreadcrumbs, + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.maxBreadcrumbs" should be of type number, got string, using default value', + ); +}); + +it('accepts valid breadcrumbs.maxBreadcrumbs number', () => { + const outOptions = parse( + { + breadcrumbs: { + maxBreadcrumbs: 50, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.maxBreadcrumbs).toEqual(50); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('warns when breadcrumbs.click is not boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + // @ts-ignore + click: 'not a boolean', + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.click).toEqual(defaultOptions().breadcrumbs.click); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.click" should be of type boolean, got string, using default value', + ); +}); + +it('warns when breadcrumbs.evaluations is not boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + // @ts-ignore + evaluations: 'not a boolean', + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.evaluations).toEqual(defaultOptions().breadcrumbs.evaluations); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.evaluations" should be of type boolean, got string, using default value', + ); +}); + +it('warns when breadcrumbs.flagChange is not boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + // @ts-ignore + flagChange: 'not a boolean', + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.flagChange).toEqual(defaultOptions().breadcrumbs.flagChange); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.flagChange" should be of type boolean, got string, using default value', + ); +}); + +it('warns when breadcrumbs.keyboardInput is not boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + // @ts-ignore + keyboardInput: 'not a boolean', + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.keyboardInput).toEqual(defaultOptions().breadcrumbs.keyboardInput); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.keyboardInput" should be of type boolean, got string, using default value', + ); +}); + +it('accepts valid breadcrumbs.click boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + click: false, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.click).toEqual(false); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('accepts valid breadcrumbs.evaluations boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + evaluations: false, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.evaluations).toEqual(false); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('accepts valid breadcrumbs.flagChange boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + flagChange: false, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.flagChange).toEqual(false); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('accepts valid breadcrumbs.keyboardInput boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + keyboardInput: false, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.keyboardInput).toEqual(false); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('warns when breadcrumbs.http is not an object', () => { + const outOptions = parse( + { + breadcrumbs: { + // @ts-ignore + http: 'not an object', + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http).toEqual(defaultOptions().breadcrumbs.http); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.http" should be of type HttpBreadCrumbOptions | false, got string, using default value', + ); +}); + +it('warns when breadcrumbs.http.instrumentFetch is not boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + http: { + // @ts-ignore + instrumentFetch: 'not a boolean', + }, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http.instrumentFetch).toEqual( + defaultOptions().breadcrumbs.http.instrumentFetch, + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.http.instrumentFetch" should be of type boolean, got string, using default value', + ); +}); + +it('warns when breadcrumbs.http.instrumentXhr is not boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + http: { + // @ts-ignore + instrumentXhr: 'not a boolean', + }, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http.instrumentXhr).toEqual( + defaultOptions().breadcrumbs.http.instrumentXhr, + ); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Config option "breadcrumbs.http.instrumentXhr" should be of type boolean, got string, using default value', + ); +}); + +it('accepts valid breadcrumbs.http.instrumentFetch boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + http: { + instrumentFetch: false, + }, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http.instrumentFetch).toEqual(false); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('accepts valid breadcrumbs.http.instrumentXhr boolean', () => { + const outOptions = parse( + { + breadcrumbs: { + http: { + instrumentXhr: false, + }, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http.instrumentXhr).toEqual(false); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('accepts valid breadcrumbs.http.customUrlFilter function', () => { + const outOptions = parse( + { + breadcrumbs: { + http: { + customUrlFilter: (url: string) => url.replace('secret', 'redacted'), + }, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http.customUrlFilter).toBeDefined(); + expect(outOptions.breadcrumbs.http.customUrlFilter?.('test-secret-123')).toBe( + 'test-redacted-123', + ); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); + +it('warns when breadcrumbs.http.customUrlFilter is not a function', () => { + const outOptions = parse( + { + breadcrumbs: { + http: { + // @ts-ignore + customUrlFilter: 'not a function', + }, + }, + }, + mockLogger, + ); + + expect(outOptions.breadcrumbs.http.customUrlFilter).toBeUndefined(); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'The "breadcrumbs.http.customUrlFilter" must be a function. Received string', + ); +}); diff --git a/packages/telemetry/browser-telemetry/src/MinLogger.ts b/packages/telemetry/browser-telemetry/src/MinLogger.ts new file mode 100644 index 000000000..dced72e95 --- /dev/null +++ b/packages/telemetry/browser-telemetry/src/MinLogger.ts @@ -0,0 +1,9 @@ +/** + * Minimal logging implementation. Compatible with an LDLogger. + * + * implementation node: Does not use a logging implementation exported by the SDK. + * This allows usage with multiple SDK versions. + */ +export interface MinLogger { + warn(...args: any[]): void; +} diff --git a/packages/telemetry/browser-telemetry/src/options.ts b/packages/telemetry/browser-telemetry/src/options.ts new file mode 100644 index 000000000..a801f5ed4 --- /dev/null +++ b/packages/telemetry/browser-telemetry/src/options.ts @@ -0,0 +1,285 @@ +import { Collector } from './api/Collector'; +import { HttpBreadCrumbOptions, Options, StackOptions, UrlFilter } from './api/Options'; +import { MinLogger } from './MinLogger'; + +export function defaultOptions(): ParsedOptions { + return { + breadcrumbs: { + maxBreadcrumbs: 50, + evaluations: true, + flagChange: true, + click: true, + keyboardInput: true, + http: { + instrumentFetch: true, + instrumentXhr: true, + }, + }, + stack: { + source: { + beforeLines: 3, + afterLines: 3, + maxLineLength: 280, + }, + }, + maxPendingEvents: 100, + collectors: [], + }; +} + +function wrongOptionType(name: string, expectedType: string, actualType: string): string { + return `Config option "${name}" should be of type ${expectedType}, got ${actualType}, using default value`; +} + +function checkBasic(type: string, name: string, logger?: MinLogger): (item: T) => boolean { + return (item: T) => { + const actualType = typeof item; + if (actualType === type) { + return true; + } + logger?.warn(wrongOptionType(name, type, actualType)); + return false; + }; +} + +function itemOrDefault(item: T | undefined, defaultValue: T, checker?: (item: T) => boolean): T { + if (item !== undefined && item !== null) { + if (!checker) { + return item; + } + if (checker(item)) { + return item; + } + } + return defaultValue; +} + +function parseHttp( + options: HttpBreadCrumbOptions | false | undefined, + defaults: ParsedHttpOptions, + logger?: MinLogger, +): ParsedHttpOptions { + if (options !== undefined && options !== false && typeof options !== 'object') { + logger?.warn( + wrongOptionType('breadcrumbs.http', 'HttpBreadCrumbOptions | false', typeof options), + ); + return defaults; + } + + if (options === false) { + return { + instrumentFetch: false, + instrumentXhr: false, + }; + } + + // Make sure that the custom filter is at least a function. + if (options?.customUrlFilter) { + if (typeof options.customUrlFilter !== 'function') { + logger?.warn( + `The "breadcrumbs.http.customUrlFilter" must be a function. Received ${typeof options.customUrlFilter}`, + ); + } + } + const customUrlFilter = + options?.customUrlFilter && typeof options?.customUrlFilter === 'function' + ? options.customUrlFilter + : undefined; + + return { + instrumentFetch: itemOrDefault( + options?.instrumentFetch, + defaults.instrumentFetch, + checkBasic('boolean', 'breadcrumbs.http.instrumentFetch', logger), + ), + instrumentXhr: itemOrDefault( + options?.instrumentXhr, + defaults.instrumentXhr, + checkBasic('boolean', 'breadcrumbs.http.instrumentXhr', logger), + ), + customUrlFilter, + }; +} + +function parseStack( + options: StackOptions | undefined, + defaults: ParsedStackOptions, + logger?: MinLogger, +): ParsedStackOptions { + return { + source: { + beforeLines: itemOrDefault( + options?.source?.beforeLines, + defaults.source.beforeLines, + checkBasic('number', 'stack.beforeLines', logger), + ), + afterLines: itemOrDefault( + options?.source?.afterLines, + defaults.source.afterLines, + checkBasic('number', 'stack.afterLines', logger), + ), + maxLineLength: itemOrDefault( + options?.source?.maxLineLength, + defaults.source.maxLineLength, + checkBasic('number', 'stack.maxLineLength', logger), + ), + }, + }; +} + +export default function parse(options: Options, logger?: MinLogger): ParsedOptions { + const defaults = defaultOptions(); + if (options.breadcrumbs) { + checkBasic('object', 'breadcrumbs', logger)(options.breadcrumbs); + } + if (options.stack) { + checkBasic('object', 'stack', logger)(options.stack); + } + return { + breadcrumbs: { + maxBreadcrumbs: itemOrDefault( + options.breadcrumbs?.maxBreadcrumbs, + defaults.breadcrumbs.maxBreadcrumbs, + checkBasic('number', 'breadcrumbs.maxBreadcrumbs', logger), + ), + evaluations: itemOrDefault( + options.breadcrumbs?.evaluations, + defaults.breadcrumbs.evaluations, + checkBasic('boolean', 'breadcrumbs.evaluations', logger), + ), + flagChange: itemOrDefault( + options.breadcrumbs?.flagChange, + defaults.breadcrumbs.flagChange, + checkBasic('boolean', 'breadcrumbs.flagChange', logger), + ), + click: itemOrDefault( + options.breadcrumbs?.click, + defaults.breadcrumbs.click, + checkBasic('boolean', 'breadcrumbs.click', logger), + ), + keyboardInput: itemOrDefault( + options.breadcrumbs?.keyboardInput, + defaults.breadcrumbs.keyboardInput, + checkBasic('boolean', 'breadcrumbs.keyboardInput', logger), + ), + http: parseHttp(options.breadcrumbs?.http, defaults.breadcrumbs.http, logger), + }, + stack: parseStack(options.stack, defaults.stack), + maxPendingEvents: itemOrDefault( + options.maxPendingEvents, + defaults.maxPendingEvents, + checkBasic('number', 'maxPendingEvents', logger), + ), + collectors: [ + ...itemOrDefault(options.collectors, defaults.collectors, (item) => { + if (Array.isArray(item)) { + return true; + } + logger?.warn(logger?.warn(wrongOptionType('collectors', 'Collector[]', typeof item))); + return false; + }), + ], + }; +} + +/** + * Internal type for parsed http options. + * @internal + */ +export interface ParsedHttpOptions { + /** + * True to instrument fetch and enable fetch breadcrumbs. + */ + instrumentFetch: boolean; + + /** + * True to instrument XMLHttpRequests and enable XMLHttpRequests breadcrumbs. + */ + instrumentXhr: boolean; + + /** + * Optional custom URL filter. + */ + customUrlFilter?: UrlFilter; +} + +/** + * Internal type for parsed stack options. + * @internal + */ +export interface ParsedStackOptions { + source: { + /** + * The number of lines captured before the originating line. + */ + beforeLines: number; + + /** + * The number of lines captured after the originating line. + */ + afterLines: number; + + /** + * The maximum length of source line to include. Lines longer than this will be + * trimmed. + */ + maxLineLength: number; + }; +} + +/** + * Internal type for parsed options. + * @internal + */ +export interface ParsedOptions { + /** + * The maximum number of pending events. Events may be captured before the LaunchDarkly + * SDK is initialized and these are stored until they can be sent. This only affects the + * events captured during initialization. + */ + maxPendingEvents: number; + /** + * Properties related to automatic breadcrumb collection. + */ + breadcrumbs: { + /** + * Set the maximum number of breadcrumbs. Defaults to 50. + */ + maxBreadcrumbs: number; + + /** + * True to enable automatic evaluation breadcrumbs. Defaults to true. + */ + evaluations: boolean; + + /** + * True to enable flag change breadcrumbs. Defaults to true. + */ + flagChange: boolean; + + /** + * True to enable click breadcrumbs. Defaults to true. + */ + click: boolean; + + /** + * True to enable input breadcrumbs for keypresses. Defaults to true. + */ + keyboardInput?: boolean; + + /** + * Settings for http instrumentation and breadcrumbs. + */ + http: ParsedHttpOptions; + }; + + /** + * Settings which affect call stack capture. + */ + stack: ParsedStackOptions; + + /** + * Additional, or custom, collectors. + */ + collectors: Collector[]; +} From 0ecd1d28b904e9928e2a6a9908da8f39c9383d9a Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 8 Nov 2024 14:12:13 -0800 Subject: [PATCH 2/6] feat: Add stack trace parsing. --- .../__tests__/stack/StackParser.test.ts | 121 ++++++++++++++ .../src/stack/StackParser.ts | 154 ++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts create mode 100644 packages/telemetry/browser-telemetry/src/stack/StackParser.ts diff --git a/packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts b/packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts new file mode 100644 index 000000000..0fdcbd445 --- /dev/null +++ b/packages/telemetry/browser-telemetry/__tests__/stack/StackParser.test.ts @@ -0,0 +1,121 @@ +import { + getLines, + getSrcLines, + processUrlToFileName, + TrimOptions, + trimSourceLine, +} from '../../src/stack/StackParser'; + +it.each([ + ['http://www.launchdarkly.com', 'http://www.launchdarkly.com/', '(index)'], + ['http://www.launchdarkly.com', 'http://www.launchdarkly.com/test/(index)', 'test/(index)'], + ['http://www.launchdarkly.com', 'http://www.launchdarkly.com/test.js', 'test.js'], + ['http://localhost:8080', 'http://localhost:8080/dist/main.js', 'dist/main.js'], +])('handles URL parsing to file names', (origin: string, url: string, expected: string) => { + expect(processUrlToFileName(url, origin)).toEqual(expected); +}); + +it.each([ + ['this is the source line', 5, { maxLength: 10, beforeColumnCharacters: 2 }, 's is the s'], + ['this is the source line', 0, { maxLength: 10, beforeColumnCharacters: 2 }, 'this is th'], + ['this is the source line', 2, { maxLength: 10, beforeColumnCharacters: 0 }, 'is is the '], + ['12345', 0, { maxLength: 5, beforeColumnCharacters: 2 }, '12345'], + ['this is the source line', 21, { maxLength: 10, beforeColumnCharacters: 2 }, 'line'], +])( + 'trims source lines', + (source: string, column: number, options: TrimOptions, expected: string) => { + expect(trimSourceLine(options, source, column)).toEqual(expected); + }, +); + +describe('given source lines', () => { + const lines = ['1234567890', 'ABCDEFGHIJ', '0987654321', 'abcdefghij']; + + it('can get a range which would underflow the lines', () => { + expect(getLines(-1, 2, lines, (input) => input)).toStrictEqual(['1234567890', 'ABCDEFGHIJ']); + }); + + it('can get a range which would overflow the lines', () => { + expect(getLines(2, 4, lines, (input) => input)).toStrictEqual(['0987654321', 'abcdefghij']); + }); + + it('can get a range which is satisfied by the lines', () => { + expect(getLines(0, 4, lines, (input) => input)).toStrictEqual([ + '1234567890', + 'ABCDEFGHIJ', + '0987654321', + 'abcdefghij', + ]); + }); +}); + +describe('given an input stack frame', () => { + const inputFrame = { + context: ['1234567890', 'ABCDEFGHIJ', 'the src line', '0987654321', 'abcdefghij'], + column: 0, + }; + + it('can produce a full stack source in the output frame', () => { + expect( + getSrcLines(inputFrame, { + source: { + beforeLines: 2, + afterLines: 2, + maxLineLength: 280, + }, + }), + ).toMatchObject({ + srcBefore: ['1234567890', 'ABCDEFGHIJ'], + srcLine: 'the src line', + srcAfter: ['0987654321', 'abcdefghij'], + }); + }); + + it('can trim all the lines', () => { + expect( + getSrcLines(inputFrame, { + source: { + beforeLines: 2, + afterLines: 2, + maxLineLength: 1, + }, + }), + ).toMatchObject({ + srcBefore: ['1', 'A'], + srcLine: 't', + srcAfter: ['0', 'a'], + }); + }); + + it('can handle fewer input lines than the expected context', () => { + expect( + getSrcLines(inputFrame, { + source: { + beforeLines: 3, + afterLines: 3, + maxLineLength: 280, + }, + }), + ).toMatchObject({ + srcBefore: ['1234567890', 'ABCDEFGHIJ'], + srcLine: 'the src line', + srcAfter: ['0987654321', 'abcdefghij'], + }); + }); + + it('can handle more input lines than the expected context', () => { + expect( + getSrcLines(inputFrame, { + source: { + beforeLines: 1, + afterLines: 1, + maxLineLength: 280, + }, + }), + ).toMatchObject({ + srcBefore: ['ABCDEFGHIJ'], + srcLine: 'the src line', + srcAfter: ['0987654321'], + }); + }); +}); diff --git a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts new file mode 100644 index 000000000..64f3f33ba --- /dev/null +++ b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts @@ -0,0 +1,154 @@ +import { computeStackTrace } from 'tracekit'; + +import { StackFrame } from '../api/stack/StackFrame'; +import { StackTrace } from '../api/stack/StackTrace'; +import { ParsedStackOptions } from '../options'; + +const INDEX_SPECIFIER = '(index)'; + +/** + * For files hosted on the origin attempt to reduce to just a filename. + * If the origin matches the source file, then the special identifier `(index)` will + * be used. + * + * @param input The input URL. + * @returns The output file name. + */ +export function processUrlToFileName(input: string, origin: string): string { + let cleaned = input; + if (input.startsWith(origin)) { + cleaned = input.slice(origin.length); + if (cleaned.startsWith('/')) { + cleaned = cleaned.slice(1); + } + if (cleaned === '') { + cleaned = INDEX_SPECIFIER; + } + if (cleaned.endsWith('/')) { + cleaned += INDEX_SPECIFIER; + } + } + return cleaned; +} + +export interface TrimOptions { + /** + * The maximum length of the trimmed line. + */ + maxLength: number; + + /** + * If the line needs trimmed, then this is the number of character to retain before the + * originating character of the frame. + */ + beforeColumnCharacters: number; +} + +/** + * Trim a source string to a reasonable size. + * + * @param options Configuration which affects trimming. + * @param line The source code line to trim. + * @param column The column which the stack frame originates from. + * @returns A trimmed source string. + */ +export function trimSourceLine(options: TrimOptions, line: string, column: number): string { + if (line.length <= options.maxLength) { + return line; + } + const captureStart = Math.max(0, column - options.beforeColumnCharacters); + const captureEnd = Math.min(line.length, captureStart + options.maxLength); + return line.slice(captureStart, captureEnd); +} + +/** + * Exported for testing. + */ +export function getLines( + start: number, + end: number, + context: string[], + trimmer: (val: string) => string, +): string[] { + const adjustedStart = start < 0 ? 0 : start; + const adjustedEnd = end > context.length ? context.length : end; + if (adjustedStart < adjustedEnd) { + return context.slice(adjustedStart, adjustedEnd).map(trimmer); + } + return []; +} + +/** + * Exported for testing. + */ +export function getSrcLines( + inFrame: { + // Tracekit returns null potentially. We accept undefined as well to be as lenient here + // as we can. + context?: string[] | null; + column?: number | null; + }, + options: ParsedStackOptions, +): { + srcBefore?: string[]; + srcLine?: string; + srcAfter?: string[]; +} { + const { context } = inFrame; + // It should be present, but we don't want to trust that it is. + if (!context) { + return {}; + } + const { maxLineLength } = options.source; + const beforeColumnCharacters = Math.floor(maxLineLength / 2); + + // The before and after lines will not be precise while we use TraceKit. + // By forking it we should be able to achieve a more optimal result. + + // Trimmer for non-origin lines. Starts at column 0. + const trimmer = (input: string) => + trimSourceLine( + { + maxLength: options.source.maxLineLength, + beforeColumnCharacters, + }, + input, + 0, + ); + + const origin = Math.floor(context.length / 2); + return { + srcBefore: getLines(origin - options.source.beforeLines, origin, context, trimmer), + srcLine: trimSourceLine( + { + maxLength: maxLineLength, + beforeColumnCharacters, + }, + context[origin], + inFrame.column || 0, + ), + srcAfter: getLines(origin + 1, origin + 1 + options.source.afterLines, context, trimmer), + }; +} + +/** + * Parse the browser stack trace into a StackTrace which contains frames with specific fields parsed + * from the free-form stack. Browser stack traces are not standardized, so implementations handling + * the output should be resilient to missing fields. + * + * @param error The error to generate a StackTrace for. + * @returns The stack trace for the given error. + */ +export default function parse(error: Error, options: ParsedStackOptions): StackTrace { + const parsed = computeStackTrace(error); + const frames: StackFrame[] = parsed.stack.reverse().map((inFrame) => ({ + fileName: processUrlToFileName(inFrame.url, window.location.origin), + function: inFrame.func, + line: inFrame.line, + col: inFrame.column, + ...getSrcLines(inFrame, options), + })); + return { + frames, + }; +} From 5c1fffc1bce0ac8885c937fe1cf84010947af8d7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 14 Nov 2024 10:34:40 -0800 Subject: [PATCH 3/6] Update packages/telemetry/browser-telemetry/src/stack/StackParser.ts Co-authored-by: Casey Waldren --- packages/telemetry/browser-telemetry/src/stack/StackParser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts index 64f3f33ba..a88e3e790 100644 --- a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts +++ b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts @@ -38,7 +38,7 @@ export interface TrimOptions { maxLength: number; /** - * If the line needs trimmed, then this is the number of character to retain before the + * If the line needs to be trimmed, then this is the number of character to retain before the * originating character of the frame. */ beforeColumnCharacters: number; From 89c3a76a38cd761e927be3cd3cd6090c9a91690f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:23:58 -0800 Subject: [PATCH 4/6] PR comments. --- .../src/stack/StackParser.ts | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts index a88e3e790..915d28ed5 100644 --- a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts +++ b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts @@ -4,6 +4,16 @@ import { StackFrame } from '../api/stack/StackFrame'; import { StackTrace } from '../api/stack/StackTrace'; import { ParsedStackOptions } from '../options'; +/** + * In the browser we will not always be able to determine the source file that code originates + * from. When you access a route it may just return HTML with embedded source, or just source, + * in which case there may not be a file name. + * + * There will also be cases where there is no source file, such as when running with various + * dev servers. + * + * In these situations we use this constant in place of the file name. + */ const INDEX_SPECIFIER = '(index)'; /** @@ -18,12 +28,19 @@ export function processUrlToFileName(input: string, origin: string): string { let cleaned = input; if (input.startsWith(origin)) { cleaned = input.slice(origin.length); + // If the input is a single `/` then it would get removed and we would + // be left with an empty string. That empty string would get replaced with + // the INDEX_SPECIFIER. In cases where a `/` remains, either singular + // or at the end of a path, then we will append the index specifier. + // For instance the route `/test/` would ultimately be `test/(index)`. if (cleaned.startsWith('/')) { cleaned = cleaned.slice(1); } + if (cleaned === '') { - cleaned = INDEX_SPECIFIER; + return INDEX_SPECIFIER; } + if (cleaned.endsWith('/')) { cleaned += INDEX_SPECIFIER; } @@ -62,7 +79,27 @@ export function trimSourceLine(options: TrimOptions, line: string, column: numbe } /** + * Given a context get trimmed source lines within the specified range. + * + * The context is a list of source code lines, this function returns a subset of + * lines which have been trimmed. + * + * If an error is on a specific line of source code we want to be able to get + * lines before and after that line. This is done relative to the originating + * line of source. + * + * If you wanted to get 3 lines before the origin line, then this function would + * need to be called with `start: originLine - 3, end: originLine`. + * + * If the `start` would underflow the context, then the start is set to 0. + * If the `end` would overflow the context, then the end is set to the context + * length. + * * Exported for testing. + * + * @param start The inclusive start index. + * @param end The exclusive end index. + * @param trimmer Method which will trim individual lines. */ export function getLines( start: number, @@ -79,6 +116,13 @@ export function getLines( } /** + * Given a stack frame produce source context about that stack frame. + * + * The source context includes the source line of the stack frame, some number + * of lines before the line of the stack frame, and some number of lines + * after the stack frame. The amount of context can be controlled by the + * provided options. + * * Exported for testing. */ export function getSrcLines( From a886e9289d2cdaefbc3babfa4c4da1f1759f531e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:25:32 -0800 Subject: [PATCH 5/6] More comments. --- packages/telemetry/browser-telemetry/src/stack/StackParser.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts index 915d28ed5..68417cdfa 100644 --- a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts +++ b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts @@ -148,6 +148,8 @@ export function getSrcLines( // The before and after lines will not be precise while we use TraceKit. // By forking it we should be able to achieve a more optimal result. + // We only need to do this if we are not getting sufficient quality using this + // method. // Trimmer for non-origin lines. Starts at column 0. const trimmer = (input: string) => From 2509315b4c73e69ddb6ff6094952854560a9ed88 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:28:20 -0800 Subject: [PATCH 6/6] More comments --- .../telemetry/browser-telemetry/src/stack/StackParser.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts index 68417cdfa..89b88ab86 100644 --- a/packages/telemetry/browser-telemetry/src/stack/StackParser.ts +++ b/packages/telemetry/browser-telemetry/src/stack/StackParser.ts @@ -152,6 +152,13 @@ export function getSrcLines( // method. // Trimmer for non-origin lines. Starts at column 0. + // Non-origin lines are lines which are not the line for a specific stack + // frame, but instead the lines before or after that frame. + // ``` + // console.log("before origin"); // non-origin line + // throw new Error("this is the origin"); // origin line + // console.log("after origin); // non-origin line + // ``` const trimmer = (input: string) => trimSourceLine( { @@ -164,6 +171,7 @@ export function getSrcLines( const origin = Math.floor(context.length / 2); return { + // The lines immediately preceeding the origin line. srcBefore: getLines(origin - options.source.beforeLines, origin, context, trimmer), srcLine: trimSourceLine( { @@ -173,6 +181,7 @@ export function getSrcLines( context[origin], inFrame.column || 0, ), + // The lines immediately following the origin line. srcAfter: getLines(origin + 1, origin + 1 + options.source.afterLines, context, trimmer), }; }