From 31935f023d5194392e85c5c554846cf3eefee0b2 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sun, 20 Jun 2021 22:20:16 +0100 Subject: [PATCH 1/3] node stackwalk for Electron --- packages/node/src/backend.ts | 88 +++--------------------- packages/node/src/eventbuilder.ts | 106 +++++++++++++++++++++++++++++ packages/node/src/index.ts | 1 + packages/node/src/parsers.ts | 41 +++++++---- packages/node/test/parsers.test.ts | 16 ++--- 5 files changed, 150 insertions(+), 102 deletions(-) create mode 100644 packages/node/src/eventbuilder.ts diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index 8220d690ceb4..8a2bdb14df4e 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -1,17 +1,9 @@ -import { BaseBackend, getCurrentHub } from '@sentry/core'; -import { Event, EventHint, Mechanism, Severity, Transport, TransportOptions } from '@sentry/types'; -import { - addExceptionMechanism, - addExceptionTypeValue, - Dsn, - extractExceptionKeysForMessage, - isError, - isPlainObject, - normalizeToSize, - SyncPromise, -} from '@sentry/utils'; +import { BaseBackend } from '@sentry/core'; +import { Event, EventHint, Severity, Transport, TransportOptions } from '@sentry/types'; +import { Dsn } from '@sentry/utils'; +import { readFile } from 'fs'; -import { extractStackFromError, parseError, parseStack, prepareFramesForEvent } from './parsers'; +import { eventFromException, eventFromMessage } from './eventbuilder'; import { HTTPSTransport, HTTPTransport } from './transports'; import { NodeOptions } from './types'; @@ -24,79 +16,15 @@ export class NodeBackend extends BaseBackend { * @inheritDoc */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types - public eventFromException(exception: any, hint?: EventHint): PromiseLike { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let ex: any = exception; - const providedMechanism: Mechanism | undefined = - hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism; - const mechanism: Mechanism = providedMechanism || { - handled: true, - type: 'generic', - }; - - if (!isError(exception)) { - if (isPlainObject(exception)) { - // This will allow us to group events based on top-level keys - // which is much better than creating new group when any key/value change - const message = `Non-Error exception captured with keys: ${extractExceptionKeysForMessage(exception)}`; - - getCurrentHub().configureScope(scope => { - scope.setExtra('__serialized__', normalizeToSize(exception as Record)); - }); - - ex = (hint && hint.syntheticException) || new Error(message); - (ex as Error).message = message; - } else { - // This handles when someone does: `throw "something awesome";` - // We use synthesized Error here so we can extract a (rough) stack trace. - ex = (hint && hint.syntheticException) || new Error(exception as string); - (ex as Error).message = exception; - } - mechanism.synthetic = true; - } - - return new SyncPromise((resolve, reject) => - parseError(ex as Error, this._options) - .then(event => { - addExceptionTypeValue(event, undefined, undefined); - addExceptionMechanism(event, mechanism); - - resolve({ - ...event, - event_id: hint && hint.event_id, - }); - }) - .then(null, reject), - ); + public eventFromException(exception: any, hint?: EventHint | undefined): PromiseLike { + return eventFromException(this._options, exception, readFile, hint); } /** * @inheritDoc */ public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: EventHint): PromiseLike { - const event: Event = { - event_id: hint && hint.event_id, - level, - message, - }; - - return new SyncPromise(resolve => { - if (this._options.attachStacktrace && hint && hint.syntheticException) { - const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; - void parseStack(stack, this._options) - .then(frames => { - event.stacktrace = { - frames: prepareFramesForEvent(frames), - }; - resolve(event); - }) - .then(null, () => { - resolve(event); - }); - } else { - resolve(event); - } - }); + return eventFromMessage(this._options, message, level, readFile, hint); } /** diff --git a/packages/node/src/eventbuilder.ts b/packages/node/src/eventbuilder.ts new file mode 100644 index 000000000000..7ac5868c9227 --- /dev/null +++ b/packages/node/src/eventbuilder.ts @@ -0,0 +1,106 @@ +import { getCurrentHub } from '@sentry/core'; +import { Event, EventHint, Mechanism, Severity } from '@sentry/types'; +import { + addExceptionMechanism, + addExceptionTypeValue, + extractExceptionKeysForMessage, + isError, + isPlainObject, + normalizeToSize, + SyncPromise, +} from '@sentry/utils'; + +import { extractStackFromError, parseError, parseStack, prepareFramesForEvent, ReadFileFn } from './parsers'; +import { NodeOptions } from './types'; + +/** + * Builds and Event from a Exception + * @hidden + */ +export function eventFromException( + options: NodeOptions, + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types + exception: any, + readFile?: ReadFileFn, + hint?: EventHint, +): PromiseLike { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let ex: any = exception; + const providedMechanism: Mechanism | undefined = + hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism; + const mechanism: Mechanism = providedMechanism || { + handled: true, + type: 'generic', + }; + + if (!isError(exception)) { + if (isPlainObject(exception)) { + // This will allow us to group events based on top-level keys + // which is much better than creating new group when any key/value change + const message = `Non-Error exception captured with keys: ${extractExceptionKeysForMessage(exception)}`; + + getCurrentHub().configureScope(scope => { + scope.setExtra('__serialized__', normalizeToSize(exception as Record)); + }); + + ex = (hint && hint.syntheticException) || new Error(message); + (ex as Error).message = message; + } else { + // This handles when someone does: `throw "something awesome";` + // We use synthesized Error here so we can extract a (rough) stack trace. + ex = (hint && hint.syntheticException) || new Error(exception as string); + (ex as Error).message = exception; + } + mechanism.synthetic = true; + } + + return new SyncPromise((resolve, reject) => + parseError(ex as Error, readFile, options) + .then(event => { + addExceptionTypeValue(event, undefined, undefined); + addExceptionMechanism(event, mechanism); + + resolve({ + ...event, + event_id: hint && hint.event_id, + }); + }) + .then(null, reject), + ); +} + +/** + * Builds and Event from a Message + * @hidden + */ +export function eventFromMessage( + options: NodeOptions, + message: string, + level: Severity = Severity.Info, + readFile?: ReadFileFn, + hint?: EventHint, +): PromiseLike { + const event: Event = { + event_id: hint && hint.event_id, + level, + message, + }; + + return new SyncPromise(resolve => { + if (options.attachStacktrace && hint && hint.syntheticException) { + const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; + void parseStack(stack, readFile, options) + .then(frames => { + event.stacktrace = { + frames: prepareFramesForEvent(frames), + }; + resolve(event); + }) + .then(null, () => { + resolve(event); + }); + } else { + resolve(event); + } + }); +} diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index b3bae88dfa8a..98910f4ce2b5 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -42,6 +42,7 @@ export { NodeOptions } from './types'; export { NodeBackend } from './backend'; export { NodeClient } from './client'; export { defaultIntegrations, init, lastEventId, flush, close, getSentryRelease } from './sdk'; +export { eventFromException, eventFromMessage } from './eventbuilder'; export { deepReadDirSync } from './utils'; export { SDK_NAME } from './version'; diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index e9dbfb9ebb39..208c5d740095 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,6 +1,5 @@ import { Event, Exception, ExtendedError, StackFrame } from '@sentry/types'; import { addContextToFrame, basename, dirname, SyncPromise } from '@sentry/utils'; -import { readFile } from 'fs'; import { LRUMap } from 'lru_map'; import * as stacktrace from './stacktrace'; @@ -9,6 +8,8 @@ import { NodeOptions } from './types'; const DEFAULT_LINES_OF_CONTEXT: number = 7; const FILE_CONTENT_CACHE = new LRUMap(100); +export type ReadFileFn = (path: string, callback: (err: Error | null, data: Buffer) => void) => void; + /** * Resets the file cache. Exists for testing purposes. * @hidden @@ -68,7 +69,7 @@ function getModule(filename: string, base?: string): string { * * @param filenames Array of filepaths to read content from. */ -function readSourceFiles(filenames: string[]): PromiseLike<{ [key: string]: string | null }> { +function readSourceFiles(filenames: string[], readFile: ReadFileFn): PromiseLike<{ [key: string]: string | null }> { // we're relying on filenames being de-duped already if (filenames.length === 0) { return SyncPromise.resolve({}); @@ -135,7 +136,11 @@ export function extractStackFromError(error: Error): stacktrace.StackFrame[] { /** * @hidden */ -export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): PromiseLike { +export function parseStack( + stack: stacktrace.StackFrame[], + readFile?: ReadFileFn, + options?: NodeOptions, +): PromiseLike { const filesToRead: string[] = []; const linesOfContext = @@ -179,13 +184,16 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions return SyncPromise.resolve(frames); } - try { - return addPrePostContext(filesToRead, frames, linesOfContext); - } catch (_) { - // This happens in electron for example where we are not able to read files from asar. - // So it's fine, we recover be just returning all frames without pre/post context. - return SyncPromise.resolve(frames); + if (readFile) { + try { + return addPrePostContext(filesToRead, frames, linesOfContext, readFile); + } catch (_) { + // This happens in electron for example where we are not able to read files from asar. + // So it's fine, we recover be just returning all frames without pre/post context. + } } + + return SyncPromise.resolve(frames); } /** @@ -198,9 +206,10 @@ function addPrePostContext( filesToRead: string[], frames: StackFrame[], linesOfContext: number, + readFile: ReadFileFn, ): PromiseLike { return new SyncPromise(resolve => - readSourceFiles(filesToRead).then(sourceFiles => { + readSourceFiles(filesToRead, readFile).then(sourceFiles => { const result = frames.map(frame => { if (frame.filename && sourceFiles[frame.filename]) { try { @@ -223,11 +232,15 @@ function addPrePostContext( /** * @hidden */ -export function getExceptionFromError(error: Error, options?: NodeOptions): PromiseLike { +export function getExceptionFromError( + error: Error, + readFile?: ReadFileFn, + options?: NodeOptions, +): PromiseLike { const name = error.name || error.constructor.name; const stack = extractStackFromError(error); return new SyncPromise(resolve => - parseStack(stack, options).then(frames => { + parseStack(stack, readFile, options).then(frames => { const result = { stacktrace: { frames: prepareFramesForEvent(frames), @@ -243,9 +256,9 @@ export function getExceptionFromError(error: Error, options?: NodeOptions): Prom /** * @hidden */ -export function parseError(error: ExtendedError, options?: NodeOptions): PromiseLike { +export function parseError(error: ExtendedError, readFile?: ReadFileFn, options?: NodeOptions): PromiseLike { return new SyncPromise(resolve => - getExceptionFromError(error, options).then((exception: Exception) => { + getExceptionFromError(error, readFile, options).then((exception: Exception) => { resolve({ exception: { values: [exception], diff --git a/packages/node/test/parsers.test.ts b/packages/node/test/parsers.test.ts index 3097b85d33c7..501417e4d0f4 100644 --- a/packages/node/test/parsers.test.ts +++ b/packages/node/test/parsers.test.ts @@ -22,10 +22,10 @@ describe('parsers.ts', () => { test('parseStack with same file', done => { expect.assertions(1); let mockCalls = 0; - void Parsers.parseStack(frames) + void Parsers.parseStack(frames, fs.readFile) .then(_ => { mockCalls = spy.mock.calls.length; - void Parsers.parseStack(frames) + void Parsers.parseStack(frames, fs.readFile) .then(_1 => { // Calls to readFile shouldn't increase if there isn't a new error expect(spy).toHaveBeenCalledTimes(mockCalls); @@ -54,7 +54,7 @@ describe('parsers.ts', () => { typeName: 'module.exports../src/index.ts', }, ]; - return Parsers.parseStack(framesWithFilePath).then(_ => { + return Parsers.parseStack(framesWithFilePath, fs.readFile).then(_ => { expect(spy).toHaveBeenCalledTimes(1); }); }); @@ -63,14 +63,14 @@ describe('parsers.ts', () => { expect.assertions(2); let mockCalls = 0; let newErrorCalls = 0; - void Parsers.parseStack(frames) + void Parsers.parseStack(frames, fs.readFile) .then(_ => { mockCalls = spy.mock.calls.length; - void Parsers.parseStack(stacktrace.parse(getError())) + void Parsers.parseStack(stacktrace.parse(getError()), fs.readFile) .then(_1 => { newErrorCalls = spy.mock.calls.length; expect(newErrorCalls).toBeGreaterThan(mockCalls); - void Parsers.parseStack(stacktrace.parse(getError())) + void Parsers.parseStack(stacktrace.parse(getError()), fs.readFile) .then(_2 => { expect(spy).toHaveBeenCalledTimes(newErrorCalls); done(); @@ -120,14 +120,14 @@ describe('parsers.ts', () => { typeName: 'module.exports../src/index.ts', }, ]; - return Parsers.parseStack(framesWithDuplicateFiles).then(_ => { + return Parsers.parseStack(framesWithDuplicateFiles, fs.readFile).then(_ => { expect(spy).toHaveBeenCalledTimes(1); }); }); test('parseStack with no context', async () => { expect.assertions(1); - return Parsers.parseStack(frames, { frameContextLines: 0 }).then(_ => { + return Parsers.parseStack(frames, fs.readFile, { frameContextLines: 0 }).then(_ => { expect(spy).toHaveBeenCalledTimes(0); }); }); From f143c1014ef4211b7c626dc7e0dc5523c9e1682b Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 21 Jun 2021 12:22:56 +0100 Subject: [PATCH 2/3] Re-order params --- packages/node/src/backend.ts | 6 +++--- packages/node/src/eventbuilder.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index 8a2bdb14df4e..fdcf15ed490a 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -16,15 +16,15 @@ export class NodeBackend extends BaseBackend { * @inheritDoc */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types - public eventFromException(exception: any, hint?: EventHint | undefined): PromiseLike { - return eventFromException(this._options, exception, readFile, hint); + public eventFromException(exception: any, hint?: EventHint): PromiseLike { + return eventFromException(this._options, exception, hint, readFile); } /** * @inheritDoc */ public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: EventHint): PromiseLike { - return eventFromMessage(this._options, message, level, readFile, hint); + return eventFromMessage(this._options, message, level, hint, readFile); } /** diff --git a/packages/node/src/eventbuilder.ts b/packages/node/src/eventbuilder.ts index 7ac5868c9227..320e29acd326 100644 --- a/packages/node/src/eventbuilder.ts +++ b/packages/node/src/eventbuilder.ts @@ -21,8 +21,8 @@ export function eventFromException( options: NodeOptions, // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types exception: any, - readFile?: ReadFileFn, hint?: EventHint, + readFile?: ReadFileFn, ): PromiseLike { // eslint-disable-next-line @typescript-eslint/no-explicit-any let ex: any = exception; @@ -77,8 +77,8 @@ export function eventFromMessage( options: NodeOptions, message: string, level: Severity = Severity.Info, - readFile?: ReadFileFn, hint?: EventHint, + readFile?: ReadFileFn, ): PromiseLike { const event: Event = { event_id: hint && hint.event_id, From 8b30a9a107c517365720aa58fc35a204b634a591 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 21 Jun 2021 13:07:27 +0100 Subject: [PATCH 3/3] Move source loading to seperate file --- packages/node/src/backend.ts | 6 +- packages/node/src/eventbuilder.ts | 10 +-- packages/node/src/parsers.ts | 124 +++-------------------------- packages/node/src/sources.ts | 105 ++++++++++++++++++++++++ packages/node/test/parsers.test.ts | 19 ++--- 5 files changed, 136 insertions(+), 128 deletions(-) create mode 100644 packages/node/src/sources.ts diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index fdcf15ed490a..e8fc14a1d092 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -1,9 +1,9 @@ import { BaseBackend } from '@sentry/core'; import { Event, EventHint, Severity, Transport, TransportOptions } from '@sentry/types'; import { Dsn } from '@sentry/utils'; -import { readFile } from 'fs'; import { eventFromException, eventFromMessage } from './eventbuilder'; +import { readFilesAddPrePostContext } from './sources'; import { HTTPSTransport, HTTPTransport } from './transports'; import { NodeOptions } from './types'; @@ -17,14 +17,14 @@ export class NodeBackend extends BaseBackend { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types public eventFromException(exception: any, hint?: EventHint): PromiseLike { - return eventFromException(this._options, exception, hint, readFile); + return eventFromException(this._options, exception, hint, readFilesAddPrePostContext); } /** * @inheritDoc */ public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: EventHint): PromiseLike { - return eventFromMessage(this._options, message, level, hint, readFile); + return eventFromMessage(this._options, message, level, hint, readFilesAddPrePostContext); } /** diff --git a/packages/node/src/eventbuilder.ts b/packages/node/src/eventbuilder.ts index 320e29acd326..61069cea605a 100644 --- a/packages/node/src/eventbuilder.ts +++ b/packages/node/src/eventbuilder.ts @@ -10,7 +10,7 @@ import { SyncPromise, } from '@sentry/utils'; -import { extractStackFromError, parseError, parseStack, prepareFramesForEvent, ReadFileFn } from './parsers'; +import { extractStackFromError, parseError, parseStack, prepareFramesForEvent, ReadFilesFn } from './parsers'; import { NodeOptions } from './types'; /** @@ -22,7 +22,7 @@ export function eventFromException( // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types exception: any, hint?: EventHint, - readFile?: ReadFileFn, + readFiles?: ReadFilesFn, ): PromiseLike { // eslint-disable-next-line @typescript-eslint/no-explicit-any let ex: any = exception; @@ -55,7 +55,7 @@ export function eventFromException( } return new SyncPromise((resolve, reject) => - parseError(ex as Error, readFile, options) + parseError(ex as Error, readFiles, options) .then(event => { addExceptionTypeValue(event, undefined, undefined); addExceptionMechanism(event, mechanism); @@ -78,7 +78,7 @@ export function eventFromMessage( message: string, level: Severity = Severity.Info, hint?: EventHint, - readFile?: ReadFileFn, + readFiles?: ReadFilesFn, ): PromiseLike { const event: Event = { event_id: hint && hint.event_id, @@ -89,7 +89,7 @@ export function eventFromMessage( return new SyncPromise(resolve => { if (options.attachStacktrace && hint && hint.syntheticException) { const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; - void parseStack(stack, readFile, options) + void parseStack(stack, readFiles, options) .then(frames => { event.stacktrace = { frames: prepareFramesForEvent(frames), diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index 208c5d740095..b24df476c5c3 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,22 +1,16 @@ import { Event, Exception, ExtendedError, StackFrame } from '@sentry/types'; -import { addContextToFrame, basename, dirname, SyncPromise } from '@sentry/utils'; -import { LRUMap } from 'lru_map'; +import { basename, dirname, SyncPromise } from '@sentry/utils'; import * as stacktrace from './stacktrace'; import { NodeOptions } from './types'; const DEFAULT_LINES_OF_CONTEXT: number = 7; -const FILE_CONTENT_CACHE = new LRUMap(100); -export type ReadFileFn = (path: string, callback: (err: Error | null, data: Buffer) => void) => void; - -/** - * Resets the file cache. Exists for testing purposes. - * @hidden - */ -export function resetFileContentCache(): void { - FILE_CONTENT_CACHE.clear(); -} +export type ReadFilesFn = ( + filesToRead: string[], + frames: StackFrame[], + linesOfContext: number, +) => PromiseLike; /** JSDoc */ function getFunction(frame: stacktrace.StackFrame): string { @@ -63,65 +57,6 @@ function getModule(filename: string, base?: string): string { return file; } -/** - * This function reads file contents and caches them in a global LRU cache. - * Returns a Promise filepath => content array for all files that we were able to read. - * - * @param filenames Array of filepaths to read content from. - */ -function readSourceFiles(filenames: string[], readFile: ReadFileFn): PromiseLike<{ [key: string]: string | null }> { - // we're relying on filenames being de-duped already - if (filenames.length === 0) { - return SyncPromise.resolve({}); - } - - return new SyncPromise<{ - [key: string]: string | null; - }>(resolve => { - const sourceFiles: { - [key: string]: string | null; - } = {}; - - let count = 0; - // eslint-disable-next-line @typescript-eslint/prefer-for-of - for (let i = 0; i < filenames.length; i++) { - const filename = filenames[i]; - - const cache = FILE_CONTENT_CACHE.get(filename); - // We have a cache hit - if (cache !== undefined) { - // If it's not null (which means we found a file and have a content) - // we set the content and return it later. - if (cache !== null) { - sourceFiles[filename] = cache; - } - // eslint-disable-next-line no-plusplus - count++; - // In any case we want to skip here then since we have a content already or we couldn't - // read the file and don't want to try again. - if (count === filenames.length) { - resolve(sourceFiles); - } - continue; - } - - readFile(filename, (err: Error | null, data: Buffer) => { - const content = err ? null : data.toString(); - sourceFiles[filename] = content; - - // We always want to set the cache, even to null which means there was an error reading the file. - // We do not want to try to read the file again. - FILE_CONTENT_CACHE.set(filename, content); - // eslint-disable-next-line no-plusplus - count++; - if (count === filenames.length) { - resolve(sourceFiles); - } - }); - } - }); -} - /** * @hidden */ @@ -138,7 +73,7 @@ export function extractStackFromError(error: Error): stacktrace.StackFrame[] { */ export function parseStack( stack: stacktrace.StackFrame[], - readFile?: ReadFileFn, + readFiles?: ReadFilesFn, options?: NodeOptions, ): PromiseLike { const filesToRead: string[] = []; @@ -184,9 +119,9 @@ export function parseStack( return SyncPromise.resolve(frames); } - if (readFile) { + if (readFiles) { try { - return addPrePostContext(filesToRead, frames, linesOfContext, readFile); + return readFiles(filesToRead, frames, linesOfContext); } catch (_) { // This happens in electron for example where we are not able to read files from asar. // So it's fine, we recover be just returning all frames without pre/post context. @@ -196,51 +131,18 @@ export function parseStack( return SyncPromise.resolve(frames); } -/** - * This function tries to read the source files + adding pre and post context (source code) - * to a frame. - * @param filesToRead string[] of filepaths - * @param frames StackFrame[] containg all frames - */ -function addPrePostContext( - filesToRead: string[], - frames: StackFrame[], - linesOfContext: number, - readFile: ReadFileFn, -): PromiseLike { - return new SyncPromise(resolve => - readSourceFiles(filesToRead, readFile).then(sourceFiles => { - const result = frames.map(frame => { - if (frame.filename && sourceFiles[frame.filename]) { - try { - const lines = (sourceFiles[frame.filename] as string).split('\n'); - - addContextToFrame(lines, frame, linesOfContext); - } catch (e) { - // anomaly, being defensive in case - // unlikely to ever happen in practice but can definitely happen in theory - } - } - return frame; - }); - - resolve(result); - }), - ); -} - /** * @hidden */ export function getExceptionFromError( error: Error, - readFile?: ReadFileFn, + readFiles?: ReadFilesFn, options?: NodeOptions, ): PromiseLike { const name = error.name || error.constructor.name; const stack = extractStackFromError(error); return new SyncPromise(resolve => - parseStack(stack, readFile, options).then(frames => { + parseStack(stack, readFiles, options).then(frames => { const result = { stacktrace: { frames: prepareFramesForEvent(frames), @@ -256,9 +158,9 @@ export function getExceptionFromError( /** * @hidden */ -export function parseError(error: ExtendedError, readFile?: ReadFileFn, options?: NodeOptions): PromiseLike { +export function parseError(error: ExtendedError, readFiles?: ReadFilesFn, options?: NodeOptions): PromiseLike { return new SyncPromise(resolve => - getExceptionFromError(error, readFile, options).then((exception: Exception) => { + getExceptionFromError(error, readFiles, options).then((exception: Exception) => { resolve({ exception: { values: [exception], diff --git a/packages/node/src/sources.ts b/packages/node/src/sources.ts new file mode 100644 index 000000000000..42fc97eee72f --- /dev/null +++ b/packages/node/src/sources.ts @@ -0,0 +1,105 @@ +import { StackFrame } from '@sentry/types'; +import { addContextToFrame, SyncPromise } from '@sentry/utils'; +import { readFile } from 'fs'; +import { LRUMap } from 'lru_map'; + +const FILE_CONTENT_CACHE = new LRUMap(100); + +/** + * Resets the file cache. Exists for testing purposes. + * @hidden + */ +export function resetFileContentCache(): void { + FILE_CONTENT_CACHE.clear(); +} + +/** + * This function tries to read the source files + adding pre and post context (source code) + * to a frame. + * @param filesToRead string[] of filepaths + * @param frames StackFrame[] containg all frames + */ +export function readFilesAddPrePostContext( + filesToRead: string[], + frames: StackFrame[], + linesOfContext: number, +): PromiseLike { + return new SyncPromise(resolve => + readSourceFiles(filesToRead).then(sourceFiles => { + const result = frames.map(frame => { + if (frame.filename && sourceFiles[frame.filename]) { + try { + const lines = (sourceFiles[frame.filename] as string).split('\n'); + + addContextToFrame(lines, frame, linesOfContext); + } catch (e) { + // anomaly, being defensive in case + // unlikely to ever happen in practice but can definitely happen in theory + } + } + return frame; + }); + + resolve(result); + }), + ); +} + +/** + * This function reads file contents and caches them in a global LRU cache. + * Returns a Promise filepath => content array for all files that we were able to read. + * + * @param filenames Array of filepaths to read content from. + */ +function readSourceFiles(filenames: string[]): PromiseLike<{ [key: string]: string | null }> { + // we're relying on filenames being de-duped already + if (filenames.length === 0) { + return SyncPromise.resolve({}); + } + + return new SyncPromise<{ + [key: string]: string | null; + }>(resolve => { + const sourceFiles: { + [key: string]: string | null; + } = {}; + + let count = 0; + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < filenames.length; i++) { + const filename = filenames[i]; + + const cache = FILE_CONTENT_CACHE.get(filename); + // We have a cache hit + if (cache !== undefined) { + // If it's not null (which means we found a file and have a content) + // we set the content and return it later. + if (cache !== null) { + sourceFiles[filename] = cache; + } + // eslint-disable-next-line no-plusplus + count++; + // In any case we want to skip here then since we have a content already or we couldn't + // read the file and don't want to try again. + if (count === filenames.length) { + resolve(sourceFiles); + } + continue; + } + + readFile(filename, (err: Error | null, data: Buffer) => { + const content = err ? null : data.toString(); + sourceFiles[filename] = content; + + // We always want to set the cache, even to null which means there was an error reading the file. + // We do not want to try to read the file again. + FILE_CONTENT_CACHE.set(filename, content); + // eslint-disable-next-line no-plusplus + count++; + if (count === filenames.length) { + resolve(sourceFiles); + } + }); + } + }); +} diff --git a/packages/node/test/parsers.test.ts b/packages/node/test/parsers.test.ts index 501417e4d0f4..3a6859d20a31 100644 --- a/packages/node/test/parsers.test.ts +++ b/packages/node/test/parsers.test.ts @@ -1,6 +1,7 @@ import * as fs from 'fs'; import * as Parsers from '../src/parsers'; +import * as Sources from '../src/sources'; import * as stacktrace from '../src/stacktrace'; import { getError } from './helper/error'; @@ -11,7 +12,7 @@ describe('parsers.ts', () => { beforeEach(() => { spy = jest.spyOn(fs, 'readFile'); frames = stacktrace.parse(new Error('test')); - Parsers.resetFileContentCache(); + Sources.resetFileContentCache(); }); afterEach(() => { @@ -22,10 +23,10 @@ describe('parsers.ts', () => { test('parseStack with same file', done => { expect.assertions(1); let mockCalls = 0; - void Parsers.parseStack(frames, fs.readFile) + void Parsers.parseStack(frames, Sources.readFilesAddPrePostContext) .then(_ => { mockCalls = spy.mock.calls.length; - void Parsers.parseStack(frames, fs.readFile) + void Parsers.parseStack(frames, Sources.readFilesAddPrePostContext) .then(_1 => { // Calls to readFile shouldn't increase if there isn't a new error expect(spy).toHaveBeenCalledTimes(mockCalls); @@ -54,7 +55,7 @@ describe('parsers.ts', () => { typeName: 'module.exports../src/index.ts', }, ]; - return Parsers.parseStack(framesWithFilePath, fs.readFile).then(_ => { + return Parsers.parseStack(framesWithFilePath, Sources.readFilesAddPrePostContext).then(_ => { expect(spy).toHaveBeenCalledTimes(1); }); }); @@ -63,14 +64,14 @@ describe('parsers.ts', () => { expect.assertions(2); let mockCalls = 0; let newErrorCalls = 0; - void Parsers.parseStack(frames, fs.readFile) + void Parsers.parseStack(frames, Sources.readFilesAddPrePostContext) .then(_ => { mockCalls = spy.mock.calls.length; - void Parsers.parseStack(stacktrace.parse(getError()), fs.readFile) + void Parsers.parseStack(stacktrace.parse(getError()), Sources.readFilesAddPrePostContext) .then(_1 => { newErrorCalls = spy.mock.calls.length; expect(newErrorCalls).toBeGreaterThan(mockCalls); - void Parsers.parseStack(stacktrace.parse(getError()), fs.readFile) + void Parsers.parseStack(stacktrace.parse(getError()), Sources.readFilesAddPrePostContext) .then(_2 => { expect(spy).toHaveBeenCalledTimes(newErrorCalls); done(); @@ -120,14 +121,14 @@ describe('parsers.ts', () => { typeName: 'module.exports../src/index.ts', }, ]; - return Parsers.parseStack(framesWithDuplicateFiles, fs.readFile).then(_ => { + return Parsers.parseStack(framesWithDuplicateFiles, Sources.readFilesAddPrePostContext).then(_ => { expect(spy).toHaveBeenCalledTimes(1); }); }); test('parseStack with no context', async () => { expect.assertions(1); - return Parsers.parseStack(frames, fs.readFile, { frameContextLines: 0 }).then(_ => { + return Parsers.parseStack(frames, Sources.readFilesAddPrePostContext, { frameContextLines: 0 }).then(_ => { expect(spy).toHaveBeenCalledTimes(0); }); });