8000 ref(node): Refactor node source fetching into integration (#3729) · ptim/sentry-javascript@c60556f · GitHub
[go: up one dir, main page]

Skip to content

Commit c60556f

Browse files
authored
ref(node): Refactor node source fetching into integration (getsentry#3729)
- Moves `eventFromException` and `eventFromMessage` into `eventbuilder` to match browser SDK - Moves source context loading/addition into a `ContextLines` integration - `frameContextLines` option gets copied to `ContextLines` constructor but also pulls `frameContextLines` from `NodeOptions` to avoid a breaking change. - `frameContextLines` in `NodeOptions` marked as `@deprecated` so users learn about future migration
1 parent 8bcd867 commit c60556f

File tree

10 files changed

+266
-259
lines changed

10 files changed

+266
-259
lines changed

packages/nextjs/test/index.server.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { BaseClient } from '@sentry/core';
21
import { RewriteFrames } from '@sentry/integrations';
32
import * as SentryNode from '@sentry/node';
43
import { getCurrentHub, NodeClient } from '@sentry/node';
@@ -17,7 +16,6 @@ const global = getGlobalObject();
1716
(global as typeof global & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';
1817

1918
const nodeInit = jest.spyOn(SentryNode, 'init');
20-
const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent');
2119
const logError = jest.spyOn(logger, 'error');
2220

2321
describe('Server init()', () => {
@@ -91,7 +89,7 @@ describe('Server init()', () => {
9189
expect(currentScope._tags.vercel).toBeUndefined();
9290
});
9391

94-
it('adds 404 transaction filter', () => {
92+
it('adds 404 transaction filter', async () => {
9593
init({
9694
dsn: 'https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012',
9795
tracesSampleRate: 1.0,
@@ -102,8 +100,10 @@ describe('Server init()', () => {
102100
const transaction = hub.startTransaction({ name: '/404' });
103101
transaction.finish();
104102

103+
// We need to flush because the event processor pipeline is async whereas transaction.finish() is sync.
104+
await SentryNode.flush();
105+
105106
expect(sendEvent).not.toHaveBeenCalled();
106-
expect(captureEvent.mock.results[0].value).toBeUndefined();
107107
expect(logError).toHaveBeenCalledWith(new SentryError('An event processor returned null, will not send event.'));
108108
});
109109

packages/node/src/backend.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export class NodeBackend extends BaseBackend<NodeOptions> {
1616
*/
1717
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
1818
public eventFromException(exception: any, hint?: EventHint): PromiseLike<Event> {
19-
return eventFromException(this._options, exception, hint);
19+
return eventFromException(exception, hint);
2020
}
2121

2222
/**

packages/node/src/eventbuilder.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { extractStackFromError, parseError, parseStack, prepareFramesForEvent }
1616
* Builds and Event from a Exception
1717
* @hidden
1818
*/
19-
export function eventFromException(options: Options, exception: unknown, hint?: EventHint): PromiseLike<Event> {
19+
export function eventFromException(exception: unknown, hint?: EventHint): PromiseLike<Event> {
2020
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2121
let ex: any = exception;
2222
const providedMechanism: Mechanism | undefined =
@@ -48,7 +48,7 @@ export function eventFromException(options: Options, exception: unknown, hint?:
4848
}
4949

5050
return new SyncPromise<Event>((resolve, reject) =>
51-
parseError(ex as Error, options)
51+
parseError(ex as Error)
5252
.then(event => {
5353
addExceptionTypeValue(event, undefined, undefined);
5454
addExceptionMechanism(event, mechanism);
@@ -81,16 +81,11 @@ export function eventFromMessage(
8181
return new SyncPromise<Event>(resolve => {
8282
if (options.attachStacktrace && hint && hint.syntheticException) {
8383
const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : [];
84-
void parseStack(stack, options)
85-
.then(frames => {
86-
event.stacktrace = {
87-
frames: prepareFramesForEvent(frames),
88-
};
89-
resolve(event);
90-
})
91-
.then(null, () => {
92-
resolve(event);
93-
});
84+
const frames = parseStack(stack);
85+
event.stacktrace = {
86+
frames: prepareFramesForEvent(frames),
87+
};
88+
resolve(event);
9489
} else {
9590
resolve(event);
9691
}
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { getCurrentHub } from '@sentry/core';
2+
import { Event, EventProcessor, Integration } from '@sentry/types';
3+
import { addContextToFrame } from '@sentry/utils';
4+
import { readFile } from 'fs';
5+
import { LRUMap } from 'lru_map';
6+
7+
import { NodeClient } from '../client';
8+
9+
const FILE_CONTENT_CACHE = new LRUMap<string, string | null>(100);
10+
const DEFAULT_LINES_OF_CONTEXT = 7;
11+
12+
// TODO: Replace with promisify when minimum supported node >= v8
13+
function readTextFileAsync(path: string): Promise<string> {
14+
return new Promise((resolve, reject) => {
15+
readFile(path, 'utf8', (err, data) => {
16+
if (err) reject(err);
17+
else resolve(data);
18+
});
19+
});
20+
}
21+
22+
/**
23+
* Resets the file cache. Exists for testing purposes.
24+
* @hidden
25+
*/
26+
export function resetFileContentCache(): void {
27+
FILE_CONTENT_CACHE.clear();
28+
}
29+
30+
interface ContextLinesOptions {
31+
/**
32+
* Sets the number of context lines for each frame when loading a file.
33+
* Defaults to 7.
34+
*
35+
* Set to 0 to disable loading and inclusion of source files.
36+
**/
37+
frameContextLines?: number;
38+
}
39+
40+
/** Add node modules / packages to the event */
41+
export class ContextLines implements Integration {
42+
/**
43+
* @inheritDoc
44+
*/
45+
public static id: string = 'ContextLines';
46+
47+
/**
48+
* @inheritDoc
49+
*/
50+
public name: string = ContextLines.id;
51+
52+
public constructor(private readonly _options: ContextLinesOptions = {}) {}
53+
54+
/**
55+
* @inheritDoc
56+
*/
57+
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
58+
// This is only here to copy frameContextLines from init options if it hasn't
59+
// been set via this integrations constructor.
60+
//
61+
// TODO: Remove on next major!
62+
if (this._options.frameContextLines === undefined) {
63+
const initOptions = getCurrentHub().getClient<NodeClient>()?.getOptions();
64+
// eslint-disable-next-line deprecation/deprecation
65+
this._options.frameContextLines = initOptions?.frameContextLines;
66+
}
67+
68+
const contextLines =
69+
this._options.frameContextLines !== undefined ? this._options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;
70+
71+
addGlobalEventProcessor(event => this.addSourceContext(event, contextLines));
72+
}
73+
74+
/** Processes an event and adds context lines */
75+
public async addSourceContext(event: Event, contextLines: number): Promise<Event> {
76+
const frames = event.exception?.values?.[0].stacktrace?.frames;
77+
78+
if (frames && contextLines > 0) {
79+
const filenames: Set<string> = new Set();
80+
81+
for (const frame of frames) {
82+
if (frame.filename) {
83+
filenames.add(frame.filename);
84+
}
85+
}
86+
87+
const sourceFiles = await readSourceFiles(filenames);
88+
89+
for (const frame of frames) {
90+
if (frame.filename && sourceFiles[frame.filename]) {
91+
try {
92+
const lines = (sourceFiles[frame.filename] as string).split('\n');
93+
addContextToFrame(lines, frame, contextLines);
94+
} catch (e) {
95+
// anomaly, being defensive in case
96+
// unlikely to ever happen in practice but can definitely happen in theory
97+
}
98+
}
99+
}
100+
}
101+
102+
return event;
103+
}
104+
}
105+
106+
/**
107+
* This function reads file contents and caches them in a global LRU cache.
108+
*
109+
* @param filenames Array of filepaths to read content from.
110+
*/
111+
async function readSourceFiles(filenames: Set<string>): Promise<Record<string, string | null>> {
112+
const sourceFiles: Record<string, string | null> = {};
113+
114+
for (const filename of filenames) {
115+
const cachedFile = FILE_CONTENT_CACHE.get(filename);
116+
// We have a cache hit
117+
if (cachedFile !== undefined) {
118+
// If stored value is null, it means that we already tried, but couldn't read the content of the file. Skip.
119+
if (cachedFile === null) {
120+
continue;
121+
}
122+
123+
// Otherwise content is there, so reuse cached value.
124+
sourceFiles[filename] = cachedFile;
125+
continue;
126+
}
127+
128+
let content: string | null = null;
129+
try {
130+
content = await readTextFileAsync(filename);
131+
} catch (_) {
132+
//
133+
}
134+
135+
FILE_CONTENT_CACHE.set(filename, content);
136+
sourceFiles[filename] = content;
137+
}
138+
139+
return sourceFiles;
140+
}

packages/node/src/integrations/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@ export { OnUncaughtException } from './onuncaughtexception';
44
export { OnUnhandledRejection } from './onunhandledrejection';
55
export { LinkedErrors } from './linkederrors';
66
export { Modules } from './modules';
7+
export { ContextLines } from './contextlines';

0 commit comments

Comments
 (0)
0