From c5f54b1fad19a35dc00322181650545d2961ccc4 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Mon, 11 Sep 2017 18:24:21 -0400 Subject: [PATCH] repl: remove internal frames from runtime errors 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: https://github.com/nodejs/node/pull/15351 Reviewed-By: Prince John Wesley Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Refs: https://github.com/nodejs/node/pull/9601 --- lib/repl.js | 34 +++++++-- test/fixtures/repl-pretty-stack.js | 19 +++++ .../parallel/test-repl-pretty-custom-stack.js | 70 +++++++++++++++++++ test/parallel/test-repl-pretty-stack.js | 55 +++++++++++++++ test/parallel/test-repl.js | 2 +- 5 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/repl-pretty-stack.js create mode 100644 test/parallel/test-repl-pretty-custom-stack.js create mode 100644 test/parallel/test-repl-pretty-stack.js diff --git a/lib/repl.js b/lib/repl.js index b032521993..3ed7f7dd6d 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -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)); @@ -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) { @@ -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 { diff --git a/test/fixtures/repl-pretty-stack.js b/test/fixtures/repl-pretty-stack.js new file mode 100644 index 0000000000..26e72beeca --- /dev/null +++ b/test/fixtures/repl-pretty-stack.js @@ -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(); \ No newline at end of file diff --git a/test/parallel/test-repl-pretty-custom-stack.js b/test/parallel/test-repl-pretty-custom-stack.js new file mode 100644 index 0000000000..be102c1d67 --- /dev/null +++ b/test/parallel/test-repl-pretty-custom-stack.js @@ -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); diff --git a/test/parallel/test-repl-pretty-stack.js b/test/parallel/test-repl-pretty-stack.js new file mode 100644 index 0000000000..0fc6b3ada0 --- /dev/null +++ b/test/parallel/test-repl-pretty-stack.js @@ -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); diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 6df856a20e..54ad848b5b 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -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> $/ }, ]); }