From 5babf9d26f590e5f121a7919275605586d0cb875 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 21 Jan 2019 20:45:55 +0100 Subject: [PATCH 1/3] events: show inspected error in uncaught 'error' message If there is no handler for `.emit('error', value)` and `value` is not an `Error` object, we currently just call `.toString()` on it. Almost always, using `util.inspect()` provides better information for diagnostic purposes, so prefer to use that instead. Refs: https://github.com/nodejs/help/issues/1729 --- lib/events.js | 12 ++++++++++-- test/parallel/test-event-emitter-errors.js | 17 +++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/events.js b/lib/events.js index 10cec4bd69d4d1..195c2a172f87d0 100644 --- a/lib/events.js +++ b/lib/events.js @@ -154,10 +154,11 @@ EventEmitter.prototype.emit = function emit(type, ...args) { // If there is no 'error' event listener then throw. if (doError) { - let er; + let er, stringifiedEr; if (args.length > 0) er = args[0]; if (er instanceof Error) { + stringifiedEr = er; try { const { kExpandStackSymbol } = require('internal/util'); const capture = {}; @@ -171,10 +172,17 @@ EventEmitter.prototype.emit = function emit(type, ...args) { // Note: The comments on the `throw` lines are intentional, they show // up in Node's output if this results in an unhandled exception. throw er; // Unhandled 'error' event + } else { + const { inspect } = require('internal/util/inspect'); + try { + stringifiedEr = inspect(er); + } catch { + stringifiedEr = er; + } } // At least give some kind of context to the user const errors = lazyErrors(); - const err = new errors.ERR_UNHANDLED_ERROR(er); + const err = new errors.ERR_UNHANDLED_ERROR(stringifiedEr); err.context = er; throw err; // Unhandled 'error' event } diff --git a/test/parallel/test-event-emitter-errors.js b/test/parallel/test-event-emitter-errors.js index ef2bbee93f8bfd..4ad635ef07f195 100644 --- a/test/parallel/test-event-emitter-errors.js +++ b/test/parallel/test-event-emitter-errors.js @@ -1,6 +1,7 @@ 'use strict'; const common = require('../common'); const EventEmitter = require('events'); +const util = require('util'); const EE = new EventEmitter(); @@ -9,7 +10,7 @@ common.expectsError( { code: 'ERR_UNHANDLED_ERROR', type: Error, - message: 'Unhandled error. (Accepts a string)' + message: "Unhandled error. ('Accepts a string')" } ); @@ -18,6 +19,18 @@ common.expectsError( { code: 'ERR_UNHANDLED_ERROR', type: Error, - message: 'Unhandled error. ([object Object])' + message: "Unhandled error. ({ message: 'Error!' })" + } +); + +common.expectsError( + () => EE.emit('error', { + message: 'Error!', + [util.inspect.custom]() { throw null; } + }), + { + code: 'ERR_UNHANDLED_ERROR', + type: Error, + message: "Unhandled error. ([object Object])" } ); From 108c16fb650928615d6478b1c85066844ff3478c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 21 Jan 2019 22:14:56 +0100 Subject: [PATCH 2/3] fixup! events: show inspected error in uncaught 'error' message --- test/parallel/test-event-emitter-errors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-event-emitter-errors.js b/test/parallel/test-event-emitter-errors.js index 4ad635ef07f195..13d3a98d9c0a50 100644 --- a/test/parallel/test-event-emitter-errors.js +++ b/test/parallel/test-event-emitter-errors.js @@ -26,11 +26,11 @@ common.expectsError( common.expectsError( () => EE.emit('error', { message: 'Error!', - [util.inspect.custom]() { throw null; } + [util.inspect.custom]() { throw new Error(); } }), { code: 'ERR_UNHANDLED_ERROR', type: Error, - message: "Unhandled error. ([object Object])" + message: 'Unhandled error. ([object Object])' } ); From b896404078ab40dba699efc826a6a95cfdcd8a7c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 22 Jan 2019 22:50:34 +0100 Subject: [PATCH 3/3] fixup! fixup! events: show inspected error in uncaught 'error' message --- lib/events.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/events.js b/lib/events.js index 195c2a172f87d0..fab8652ebf0457 100644 --- a/lib/events.js +++ b/lib/events.js @@ -154,11 +154,10 @@ EventEmitter.prototype.emit = function emit(type, ...args) { // If there is no 'error' event listener then throw. if (doError) { - let er, stringifiedEr; + let er; if (args.length > 0) er = args[0]; if (er instanceof Error) { - stringifiedEr = er; try { const { kExpandStackSymbol } = require('internal/util'); const capture = {}; @@ -172,14 +171,16 @@ EventEmitter.prototype.emit = function emit(type, ...args) { // Note: The comments on the `throw` lines are intentional, they show // up in Node's output if this results in an unhandled exception. throw er; // Unhandled 'error' event - } else { - const { inspect } = require('internal/util/inspect'); - try { - stringifiedEr = inspect(er); - } catch { - stringifiedEr = er; - } } + + let stringifiedEr; + const { inspect } = require('internal/util/inspect'); + try { + stringifiedEr = inspect(er); + } catch { + stringifiedEr = er; + } + // At least give some kind of context to the user const errors = lazyErrors(); const err = new errors.ERR_UNHANDLED_ERROR(stringifiedEr);