Skip to content

Commit

Permalink
ref: Extract source context + tests (#2376)
Browse files Browse the repository at this point in the history
  • Loading branch information
HazAT authored Jan 14, 2020
1 parent 5f95f58 commit 8870389
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- [utils] ref: Extract adding source context incl. tests

## 5.11.0

- [apm] fix: Always attach `contexts.trace` to finished transaction (#2353)
Expand Down
12 changes: 2 additions & 10 deletions packages/node/src/parsers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Event, Exception, ExtendedError, StackFrame } from '@sentry/types';
import { basename, dirname, snipLine, SyncPromise } from '@sentry/utils';
import { addContextToFrame, basename, dirname, SyncPromise } from '@sentry/utils';
import { readFile } from 'fs';
import { LRUMap } from 'lru_map';

Expand Down Expand Up @@ -203,15 +203,7 @@ function addPrePostContext(
try {
const lines = (sourceFiles[frame.filename] as string).split('\n');

frame.pre_context = lines
.slice(Math.max(0, (frame.lineno || 0) - (linesOfContext + 1)), (frame.lineno || 0) - 1)
.map((line: string) => snipLine(line, 0));

frame.context_line = snipLine(lines[(frame.lineno || 0) - 1], frame.colno || 0);

frame.post_context = lines
.slice(frame.lineno || 0, (frame.lineno || 0) + linesOfContext)
.map((line: string) => snipLine(line, 0));
addContextToFrame(lines, frame, linesOfContext);
} catch (e) {
// anomaly, being defensive in case
// unlikely to ever happen in practice but can definitely happen in theory
Expand Down
26 changes: 25 additions & 1 deletion packages/utils/src/misc.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Event, Integration, WrappedFunction } from '@sentry/types';
import { Event, Integration, StackFrame, WrappedFunction } from '@sentry/types';

import { isString } from './is';
import { snipLine } from './string';

/** Internal */
interface SentryGlobal {
Expand Down Expand Up @@ -426,3 +427,26 @@ export function getFunctionName(fn: unknown): string {
return defaultFunctionName;
}
}

/**
* This function adds context (pre/post/line) lines to the provided frame
*
* @param lines string[] containing all lines
* @param frame StackFrame that will be mutated
* @param linesOfContext number of context lines we want to add pre/post
*/
export function addContextToFrame(lines: string[], frame: StackFrame, linesOfContext: number = 5): void {
const lineno = frame.lineno || 0;
const maxLines = lines.length;
const sourceLine = Math.max(Math.min(maxLines, lineno - 1), 0);

frame.pre_context = lines
.slice(Math.max(0, sourceLine - linesOfContext), sourceLine)
.map((line: string) => snipLine(line, 0));

frame.context_line = snipLine(lines[Math.min(maxLines - 1, sourceLine)], frame.colno || 0);

frame.post_context = lines
.slice(Math.min(sourceLine + 1, maxLines), sourceLine + 1 + linesOfContext)
.map((line: string) => snipLine(line, 0));
}
73 changes: 72 additions & 1 deletion packages/utils/test/misc.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { getEventDescription, getGlobalObject, parseRetryAfterHeader } from '../src/misc';
import { StackFrame } from '@sentry/types';

import { addContextToFrame, getEventDescription, getGlobalObject, parseRetryAfterHeader } from '../src/misc';

describe('getEventDescription()', () => {
test('message event', () => {
Expand Down Expand Up @@ -139,3 +141,72 @@ describe('parseRetryAfterHeader', () => {
).toEqual(13 * 1000);
});
});

describe('addContextToFrame', () => {
const lines = [
'1: a',
'2: b',
'3: c',
'4: d',
'5: e',
'6: f',
'7: g',
'8: h',
'9: i',
'10: j',
'11: k',
'12: l',
'13: m',
'14: n',
];

test('start of file', () => {
const frame: StackFrame = {
lineno: 0,
};
addContextToFrame(lines, frame);
expect(frame.pre_context).toEqual([]);
expect(frame.context_line).toEqual('1: a');
expect(frame.post_context).toEqual(['2: b', '3: c', '4: d', '5: e', '6: f']);
});

test('mid of file', () => {
const frame: StackFrame = {
lineno: 4,
};
addContextToFrame(lines, frame);
expect(frame.pre_context).toEqual(['1: a', '2: b', '3: c']);
expect(frame.context_line).toEqual('4: d');
expect(frame.post_context).toEqual(['5: e', '6: f', '7: g', '8: h', '9: i']);
});

test('end of file', () => {
const frame: StackFrame = {
lineno: 14,
};
addContextToFrame(lines, frame);
expect(frame.pre_context).toEqual(['9: i', '10: j', '11: k', '12: l', '13: m']);
expect(frame.context_line).toEqual('14: n');
expect(frame.post_context).toEqual([]);
});

test('negative', () => {
const frame: StackFrame = {
lineno: -1,
};
addContextToFrame(lines, frame);
expect(frame.pre_context).toEqual([]);
expect(frame.context_line).toEqual('1: a');
expect(frame.post_context).toEqual(['2: b', '3: c', '4: d', '5: e', '6: f']);
});

test('overshoot', () => {
const frame: StackFrame = {
lineno: 999,
};
addContextToFrame(lines, frame);
expect(frame.pre_context).toEqual(['10: j', '11: k', '12: l', '13: m', '14: n']);
expect(frame.context_line).toEqual('14: n');
expect(frame.post_context).toEqual([]);
});
});

0 comments on commit 8870389

Please sign in to comment.