Skip to content

Commit

Permalink
repl: remove internal frames from runtime errors
Browse files Browse the repository at this point in the history
When a user executes code in the REPLServer which generates an
exception, there is no need to display the REPLServer internal
stack frames.

PR-URL: nodejs/node#15351
Reviewed-By: Prince John Wesley <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>

Refs: nodejs/node#9601
  • Loading branch information
lance authored and addaleax committed Oct 11, 2017
1 parent 4c1b835 commit d4bce61
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 6 deletions.
34 changes: 29 additions & 5 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,16 @@ function REPLServer(prompt,
self._domain.on('error', function debugDomainError(e) {
debug('domain error');
const top = replMap.get(self);

const pstrace = Error.prepareStackTrace;
Error.prepareStackTrace = prepareStackTrace(pstrace);
internalUtil.decorateErrorStack(e);
Error.prepareStackTrace = pstrace;
const isError = internalUtil.isError(e);
if (e instanceof SyntaxError && e.stack) {
// remove repl:line-number and stack trace
e.stack = e.stack
.replace(/^repl:\d+\r?\n/, '')
.replace(/^\s+at\s.*\n?/gm, '');
.replace(/^repl:\d+\r?\n/, '')
.replace(/^\s+at\s.*\n?/gm, '');
} else if (isError && self.replMode === exports.REPL_MODE_STRICT) {
e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/,
(_, pre, line) => pre + (line - 1));
Expand Down Expand Up @@ -375,6 +377,30 @@ function REPLServer(prompt,
};
}

function filterInternalStackFrames(error, structuredStack) {
// search from the bottom of the call stack to
// find the first frame with a null function name
if (typeof structuredStack !== 'object')
return structuredStack;
const idx = structuredStack.reverse().findIndex(
(frame) => frame.getFunctionName() === null);

// if found, get rid of it and everything below it
structuredStack = structuredStack.splice(idx + 1);
return structuredStack;
}

function prepareStackTrace(fn) {
return (error, stackFrames) => {
const frames = filterInternalStackFrames(error, stackFrames);
if (fn) {
return fn(error, frames);
}
frames.push(error);
return frames.reverse().join('\n at ');
};
}

function _parseREPLKeyword(keyword, rest) {
var cmd = this.commands[keyword];
if (cmd) {
Expand Down Expand Up @@ -942,8 +968,6 @@ function complete(line, callback) {
} else {
const evalExpr = `try { ${expr} } catch (e) {}`;
this.eval(evalExpr, this.context, 'repl', (e, obj) => {
// if (e) console.log(e);

if (obj != null) {
if (typeof obj === 'object' || typeof obj === 'function') {
try {
Expand Down
19 changes: 19 additions & 0 deletions test/fixtures/repl-pretty-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

function a() {
b();
}

function b() {
c();
}

function c() {
d(function() { throw new Error('Whoops!'); });
}

function d(f) {
f();
}

a();
70 changes: 70 additions & 0 deletions test/parallel/test-repl-pretty-custom-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');
const assert = require('assert');
const repl = require('repl');


function run({ command, expected }) {
let accum = '';

const inputStream = new common.ArrayStream();
const outputStream = new common.ArrayStream();

outputStream.write = (data) => accum += data.replace('\r', '');

const r = repl.start({
prompt: '',
input: inputStream,
output: outputStream,
terminal: false,
useColors: false
});

r.write(`${command}\n`);
assert.strictEqual(accum, expected);
r.close();
}

const origPrepareStackTrace = Error.prepareStackTrace;
Error.prepareStackTrace = (err, stack) => {
if (err instanceof SyntaxError)
return err.toString();
stack.push(err);
return stack.reverse().join('--->\n');
};

process.on('uncaughtException', (e) => {
Error.prepareStackTrace = origPrepareStackTrace;
throw e;
});

process.on('exit', () => (Error.prepareStackTrace = origPrepareStackTrace));

const tests = [
{
// test .load for a file that throws
command: `.load ${fixtures.path('repl-pretty-stack.js')}`,
expected: 'Error: Whoops!--->\nrepl:9:24--->\nd (repl:12:3)--->\nc ' +
'(repl:9:3)--->\nb (repl:6:3)--->\na (repl:3:3)\n'
},
{
command: 'let x y;',
expected: 'let x y;\n ^\n\nSyntaxError: Unexpected identifier\n'
},
{
command: 'throw new Error(\'Whoops!\')',
expected: 'Error: Whoops!\n'
},
{
command: 'foo = bar;',
expected: 'ReferenceError: bar is not defined\n'
},
// test anonymous IIFE
{
command: '(function() { throw new Error(\'Whoops!\'); })()',
expected: 'Error: Whoops!--->\nrepl:1:21\n'
}
];

tests.forEach(run);
55 changes: 55 additions & 0 deletions test/parallel/test-repl-pretty-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';
const common = require('../common');
const fixtures = require('../common/fixtures');
const assert = require('assert');
const repl = require('repl');


function run({ command, expected }) {
let accum = '';

const inputStream = new common.ArrayStream();
const outputStream = new common.ArrayStream();

outputStream.write = (data) => accum += data.replace('\r', '');

const r = repl.start({
prompt: '',
input: inputStream,
output: outputStream,
terminal: false,
useColors: false
});

r.write(`${command}\n`);
assert.strictEqual(accum, expected);
r.close();
}

const tests = [
{
// test .load for a file that throws
command: `.load ${fixtures.path('repl-pretty-stack.js')}`,
expected: 'Error: Whoops!\n at repl:9:24\n at d (repl:12:3)\n ' +
'at c (repl:9:3)\n at b (repl:6:3)\n at a (repl:3:3)\n'
},
{
command: 'let x y;',
expected: 'let x y;\n ^\n\nSyntaxError: Unexpected identifier\n\n'
},
{
command: 'throw new Error(\'Whoops!\')',
expected: 'Error: Whoops!\n'
},
{
command: 'foo = bar;',
expected: 'ReferenceError: bar is not defined\n'
},
// test anonymous IIFE
{
command: '(function() { throw new Error(\'Whoops!\'); })()',
expected: 'Error: Whoops!\n at repl:1:21\n'
}
];

tests.forEach(run);
2 changes: 1 addition & 1 deletion test/parallel/test-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function clean_up() {
function strict_mode_error_test() {
send_expect([
{ client: client_unix, send: 'ref = 1',
expect: /^ReferenceError:\sref\sis\snot\sdefined\n\s+at\srepl:1:5/ },
expect: /^ReferenceError:\sref\sis\snot\sdefined\nnode via Unix socket> $/ },
]);
}

Expand Down

0 comments on commit d4bce61

Please sign in to comment.