Skip to content

ref(node): Include source context lines for all exceptions #4734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 31 additions & 43 deletions packages/node/src/integrations/contextlines.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getCurrentHub } from '@sentry/core';
import { Event, EventProcessor, Integration } from '@sentry/types';
import { Event, EventProcessor, Integration, StackFrame } from '@sentry/types';
import { addContextToFrame } from '@sentry/utils';
import { readFile } from 'fs';
import { LRUMap } from 'lru_map';
Expand Down Expand Up @@ -73,23 +73,26 @@ export class ContextLines implements Integration {

/** Processes an event and adds context lines */
public async addSourceContext(event: Event, contextLines: number): Promise<Event> {
const frames = event.exception?.values?.[0].stacktrace?.frames;

if (frames && contextLines > 0) {
const filenames: Set<string> = new Set();

for (const frame of frames) {
if (frame.filename) {
filenames.add(frame.filename);
if (contextLines > 0 && event.exception?.values) {
for (const exception of event.exception.values) {
if (exception.stacktrace?.frames) {
await this._addSourceContextToFrames(exception.stacktrace.frames, contextLines);
}
}
}

return event;
}

const sourceFiles = await readSourceFiles(filenames);
/** Adds context lines to frames */
public async _addSourceContextToFrames(frames: StackFrame[], contextLines: number): Promise<void> {
for (const frame of frames) {
if (frame.filename) {
const sourceFile = await _readSourceFile(frame.filename);

for (const frame of frames) {
if (frame.filename && sourceFiles[frame.filename]) {
if (sourceFile) {
try {
const lines = (sourceFiles[frame.filename] as string).split('\n');
const lines = sourceFile.split('\n');
addContextToFrame(lines, frame, contextLines);
} catch (e) {
// anomaly, being defensive in case
Expand All @@ -98,43 +101,28 @@ export class ContextLines implements Integration {
}
}
}

return event;
}
}

/**
* This function reads file contents and caches them in a global LRU cache.
* Reads file contents and caches them in a global LRU cache.
*
* @param filenames Array of filepaths to read content from.
* @param filename filepath to read content from.
*/
async function readSourceFiles(filenames: Set<string>): Promise<Record<string, string | null>> {
const sourceFiles: Record<string, string | null> = {};

for (const filename of filenames) {
const cachedFile = FILE_CONTENT_CACHE.get(filename);
// We have a cache hit
if (cachedFile !== undefined) {
// If stored value is null, it means that we already tried, but couldn't read the content of the file. Skip.
if (cachedFile === null) {
continue;
}

// Otherwise content is there, so reuse cached value.
sourceFiles[filename] = cachedFile;
continue;
}

let content: string | null = null;
try {
content = await readTextFileAsync(filename);
} catch (_) {
//
}
async function _readSourceFile(filename: string): Promise<string | null> {
const cachedFile = FILE_CONTENT_CACHE.get(filename);
// We have a cache hit
if (cachedFile !== undefined) {
return cachedFile;
}

FILE_CONTENT_CACHE.set(filename, content);
sourceFiles[filename] = content;
let content: string | null = null;
try {
content = await readTextFileAsync(filename);
} catch (_) {
//
}

return sourceFiles;
FILE_CONTENT_CACHE.set(filename, content);
return content;
}
3 changes: 2 additions & 1 deletion packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export const defaultIntegrations = [
// Common
new CoreIntegrations.InboundFilters(),
new CoreIntegrations.FunctionToString(),
new ContextLines(),
// Native Wrappers
new Console(),
new Http(),
Expand All @@ -21,6 +20,8 @@ export const defaultIntegrations = [
new OnUnhandledRejection(),
// Misc
new LinkedErrors(),
// ContextLines must come after LinkedErrors so that context is added to linked errors
new ContextLines(),
];

/**
Expand Down
41 changes: 41 additions & 0 deletions packages/node/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
Scope,
} from '../src';
import { NodeBackend } from '../src/backend';
import { LinkedErrors } from '../src/integrations';

jest.mock('@sentry/core', () => {
const original = jest.requireActual('@sentry/core');
Expand Down Expand Up @@ -198,6 +199,46 @@ describe('SentryNode', () => {
}
});

test.only('capture a linked exception with pre/post context', done => {
expect.assertions(15);
getCurrentHub().bindClient(
new NodeClient({
integrations: i => [new LinkedErrors(), ...i],
beforeSend: (event: Event) => {
expect(event.exception).not.toBeUndefined();
expect(event.exception!.values![1]).not.toBeUndefined();
expect(event.exception!.values![1].stacktrace!).not.toBeUndefined();
expect(event.exception!.values![1].stacktrace!.frames![1]).not.toBeUndefined();
expect(event.exception!.values![1].stacktrace!.frames![1].pre_context).not.toBeUndefined();
expect(event.exception!.values![1].stacktrace!.frames![1].post_context).not.toBeUndefined();
expect(event.exception!.values![1].type).toBe('Error');
expect(event.exception!.values![1].value).toBe('test');

expect(event.exception!.values![0]).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1]).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1].pre_context).not.toBeUndefined();
expect(event.exception!.values![0].stacktrace!.frames![1].post_context).not.toBeUndefined();
expect(event.exception!.values![0].type).toBe('Error');
expect(event.exception!.values![0].value).toBe('cause');
done();
return null;
},
dsn,
}),
);
try {
throw new Error('test');
} catch (e) {
try {
throw new Error('cause');
} catch (c) {
e.cause = c;
captureException(e);
}
}
});

test('capture a message', done => {
expect.assertions(2);
getCurrentHub().bindClient(
Expand Down