From 1ccaf9a8f1324f6a4e995acc8f89f231072cd384 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 28 Dec 2018 14:32:16 +0100 Subject: [PATCH] repl: indicate if errors are thrown or not Currently an error is printed identical, no matter if it is just inspected or if the error is thrown inside of the REPL. This makes sure we are able to distinguish these cases. PR-URL: https://github.com/nodejs/node/pull/25253 Reviewed-By: Gus Caplan Reviewed-By: Anna Henningsen --- lib/repl.js | 7 +- test/parallel/test-repl-harmony.js | 2 +- .../parallel/test-repl-pretty-custom-stack.js | 13 +-- test/parallel/test-repl-pretty-stack.js | 14 +-- .../test-repl-tab-complete-no-warn.js | 3 +- test/parallel/test-repl-top-level-await.js | 7 +- test/parallel/test-repl-underscore.js | 4 + test/parallel/test-repl.js | 96 ++++++++++--------- 8 files changed, 83 insertions(+), 63 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 9172931280f46f..9f14e5c1c69d3c 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -436,7 +436,10 @@ function REPLServer(prompt, } if (errStack === '') { - errStack = `Thrown: ${util.inspect(e)}`; + errStack = `Thrown: ${util.inspect(e)}\n`; + } else { + const ln = errStack.endsWith('\n') ? '' : '\n'; + errStack = `Thrown:\n${errStack}${ln}`; } if (!self.underscoreErrAssigned) { @@ -444,7 +447,7 @@ function REPLServer(prompt, } const top = replMap.get(self); - top.outputStream.write(`${errStack}\n`); + top.outputStream.write(errStack); top.clearBufferedCommand(); top.lines.level = []; top.displayPrompt(); diff --git a/test/parallel/test-repl-harmony.js b/test/parallel/test-repl-harmony.js index 5867a8caff569e..6686e63fe028ed 100644 --- a/test/parallel/test-repl-harmony.js +++ b/test/parallel/test-repl-harmony.js @@ -30,7 +30,7 @@ const child = spawn(process.execPath, args); const input = '(function(){"use strict"; const y=1;y=2})()\n'; // This message will vary based on JavaScript engine, so don't check the message // contents beyond confirming that the `Error` is a `TypeError`. -const expectOut = /^> TypeError: /; +const expectOut = /^> Thrown:\nTypeError: /; child.stderr.setEncoding('utf8'); child.stderr.on('data', function(c) { diff --git a/test/parallel/test-repl-pretty-custom-stack.js b/test/parallel/test-repl-pretty-custom-stack.js index 3c758fb2f2ef13..a37cf04263433d 100644 --- a/test/parallel/test-repl-pretty-custom-stack.js +++ b/test/parallel/test-repl-pretty-custom-stack.js @@ -46,25 +46,26 @@ 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' + expected: 'Thrown:\nError: 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' + expected: 'Thrown:\n' + + 'let x y;\n ^\n\nSyntaxError: Unexpected identifier\n' }, { command: 'throw new Error(\'Whoops!\')', - expected: 'Error: Whoops!\n' + expected: 'Thrown:\nError: Whoops!\n' }, { command: 'foo = bar;', - expected: 'ReferenceError: bar is not defined\n' + expected: 'Thrown:\nReferenceError: bar is not defined\n' }, // test anonymous IIFE { command: '(function() { throw new Error(\'Whoops!\'); })()', - expected: 'Error: Whoops!--->\nrepl:1:21\n' + expected: 'Thrown:\nError: Whoops!--->\nrepl:1:21\n' } ]; diff --git a/test/parallel/test-repl-pretty-stack.js b/test/parallel/test-repl-pretty-stack.js index f4754d315df44f..e4137b84a4c2b5 100644 --- a/test/parallel/test-repl-pretty-stack.js +++ b/test/parallel/test-repl-pretty-stack.js @@ -31,25 +31,27 @@ 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' + expected: 'Thrown:\nError: 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' + expected: 'Thrown:\n' + + 'let x y;\n ^\n\nSyntaxError: Unexpected identifier\n' }, { command: 'throw new Error(\'Whoops!\')', - expected: 'Error: Whoops!\n' + expected: 'Thrown:\nError: Whoops!\n' }, { command: 'foo = bar;', - expected: 'ReferenceError: bar is not defined\n' + expected: 'Thrown:\nReferenceError: bar is not defined\n' }, // test anonymous IIFE { command: '(function() { throw new Error(\'Whoops!\'); })()', - expected: 'Error: Whoops!\n at repl:1:21\n' + expected: 'Thrown:\nError: Whoops!\n at repl:1:21\n' } ]; diff --git a/test/parallel/test-repl-tab-complete-no-warn.js b/test/parallel/test-repl-tab-complete-no-warn.js index cef2026dfba853..58d0a1332fb193 100644 --- a/test/parallel/test-repl-tab-complete-no-warn.js +++ b/test/parallel/test-repl-tab-complete-no-warn.js @@ -5,8 +5,7 @@ const ArrayStream = require('../common/arraystream'); const repl = require('repl'); const DEFAULT_MAX_LISTENERS = require('events').defaultMaxListeners; -ArrayStream.prototype.write = () => { -}; +ArrayStream.prototype.write = () => {}; const putIn = new ArrayStream(); const testMe = repl.start('', putIn); diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js index c6717a2120f45c..73023df038b2c9 100644 --- a/test/parallel/test-repl-top-level-await.js +++ b/test/parallel/test-repl-top-level-await.js @@ -118,15 +118,15 @@ async function ordinaryTests() { [ 'if (await true) { function bar() {}; }', 'undefined' ], [ 'bar', '[Function: bar]' ], [ 'if (await true) { class Bar {}; }', 'undefined' ], - [ 'Bar', 'ReferenceError: Bar is not defined', { line: 0 } ], + [ 'Bar', 'ReferenceError: Bar is not defined', { line: 1 } ], [ 'await 0; function* gen(){}', 'undefined' ], [ 'for (var i = 0; i < 10; ++i) { await i; }', 'undefined' ], [ 'i', '10' ], [ 'for (let j = 0; j < 5; ++j) { await j; }', 'undefined' ], - [ 'j', 'ReferenceError: j is not defined', { line: 0 } ], + [ 'j', 'ReferenceError: j is not defined', { line: 1 } ], [ 'gen', '[GeneratorFunction: gen]' ], [ 'return 42; await 5;', 'SyntaxError: Illegal return statement', - { line: 3 } ], + { line: 4 } ], [ 'let o = await 1, p', 'undefined' ], [ 'p', 'undefined' ], [ 'let q = 1, s = await 2', 'undefined' ], @@ -160,6 +160,7 @@ async function ctrlCTest() { { ctrl: true, name: 'c' } ]), [ 'await timeout(100000)\r', + 'Thrown:', 'Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' + 'Script execution was interrupted by `SIGINT`', PROMPT diff --git a/test/parallel/test-repl-underscore.js b/test/parallel/test-repl-underscore.js index d75a4069250771..cbf62c3b6e0308 100644 --- a/test/parallel/test-repl-underscore.js +++ b/test/parallel/test-repl-underscore.js @@ -173,10 +173,12 @@ function testError() { 'undefined', // The error, both from the original throw and the `_error` echo. + 'Thrown:', 'Error: foo', '[Error: foo]', // The sync error, with individual property echoes + 'Thrown:', /^{ Error: ENOENT: no such file or directory, scandir '.*nonexistent.*'/, /Object\.readdirSync/, /^ errno: -(2|4058),$/, @@ -191,6 +193,7 @@ function testError() { 'undefined', // The message from the original throw + 'Thrown:', 'Error: baz', /setImmediate/, /^ at/, @@ -217,6 +220,7 @@ function testError() { "'baz'", 'Expression assignment to _error now disabled.', '0', + 'Thrown:', 'Error: quux', '0' ]); diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 91be2521aeb796..89370b0f7ad464 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -25,6 +25,7 @@ const fixtures = require('../common/fixtures'); const assert = require('assert'); const net = require('net'); const repl = require('repl'); +const { inspect } = require('util'); const message = 'Read, Eval, Print Loop'; const prompt_unix = 'node via Unix socket> '; @@ -58,7 +59,7 @@ async function runReplTests(socket, prompt, tests) { expectedLine = send; while (!lineBuffer.includes('\n')) { - lineBuffer += await event(socket, 'data'); + lineBuffer += await event(socket, expect); // Cut away the initial prompt while (lineBuffer.startsWith(prompt)) @@ -124,7 +125,7 @@ const unixTests = [ const strictModeTests = [ { send: 'ref = 1', - expect: /^ReferenceError:\s/ + expect: ['Thrown:', /^ReferenceError:\s/] } ]; @@ -132,7 +133,7 @@ const errorTests = [ // Uncaught error throws and prints out { send: 'throw new Error(\'test error\');', - expect: 'Error: test error' + expect: ['Thrown:', 'Error: test error'] }, { send: "throw { foo: 'bar' };", @@ -151,7 +152,7 @@ const errorTests = [ // But passing the same string to eval() should throw { send: 'eval("function test_func() {")', - expect: /^SyntaxError: / + expect: ['Thrown:', /^SyntaxError: /] }, // Can handle multiline template literals { @@ -208,90 +209,90 @@ const errorTests = [ // should throw { send: 'JSON.parse(\'{invalid: \\\'json\\\'}\');', - expect: [/^SyntaxError: /, ''] + expect: ['Thrown:', /^SyntaxError: /] }, // End of input to JSON.parse error is special case of syntax error, // should throw { send: 'JSON.parse(\'066\');', - expect: [/^SyntaxError: /, ''] + expect: ['Thrown:', /^SyntaxError: /] }, // should throw { send: 'JSON.parse(\'{\');', - expect: [/^SyntaxError: /, ''] + expect: ['Thrown:', /^SyntaxError: /] }, // invalid RegExps are a special case of syntax error, // should throw { send: '/(/;', - expect: /^SyntaxError: / + expect: ['Thrown:', /^SyntaxError: /] }, // invalid RegExp modifiers are a special case of syntax error, // should throw (GH-4012) { send: 'new RegExp("foo", "wrong modifier");', - expect: [/^SyntaxError: /, ''] + expect: ['Thrown:', /^SyntaxError: /] }, // Strict mode syntax errors should be caught (GH-5178) { send: '(function() { "use strict"; return 0755; })()', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: '(function(a, a, b) { "use strict"; return a + b + c; })()', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: '(function() { "use strict"; with (this) {} })()', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: '(function() { "use strict"; var x; delete x; })()', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: '(function() { "use strict"; eval = 17; })()', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: '(function() { "use strict"; if (true) function f() { } })()', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, // Named functions can be used: @@ -396,11 +397,11 @@ const errorTests = [ { send: '[] \\', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, // Do not fail when a String is created with line continuation @@ -530,6 +531,7 @@ const errorTests = [ { send: 'require("internal/repl")', expect: [ + 'Thrown:', /^{ Error: Cannot find module 'internal\/repl'/, /^ at .*/, /^ at .*/, @@ -563,11 +565,11 @@ const errorTests = [ { send: 'a = 3.5e', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, // Mitigate https://github.com/nodejs/node/issues/548 @@ -583,22 +585,22 @@ const errorTests = [ { send: 'a = 3.5e', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, // Avoid emitting stack trace { send: 'a = 3.5e', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, @@ -663,11 +665,11 @@ const errorTests = [ { send: '...[]', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, // bring back the repl to prompt @@ -678,31 +680,31 @@ const errorTests = [ { send: 'console.log("Missing comma in arg list" process.version)', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: 'x = {\nfield\n{', expect: [ - '... ... {', + '... ... Thrown:', + '{', kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { send: '(2 + 3))', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, { @@ -716,11 +718,11 @@ const errorTests = [ { send: '} else {', expect: [ + 'Thrown:', kSource, kArrow, '', - /^SyntaxError: /, - '' + /^SyntaxError: / ] }, ]; @@ -844,8 +846,16 @@ function startUnixRepl() { ]); } -function event(ee, eventName) { - return new Promise((resolve) => { - ee.once(eventName, common.mustCall(resolve)); +function event(ee, expected) { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + const data = inspect(expected, { compact: false }); + const msg = `The REPL did not reply as expected for:\n\n${data}`; + reject(new Error(msg)); + }, 500); + ee.once('data', common.mustCall((...args) => { + clearTimeout(timeout); + resolve(...args); + })); }); }