From e4e4e0ac5b2d65fad8f5298caaa2154ca6a31ffe Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Sun, 10 Dec 2017 13:11:18 -0500
Subject: [PATCH 1/4] domain: fix error emit handling

Fix an issue where error is never emitted on the original EventEmitter
in situations where a listener for error does exist.

Refactor to eliminate unnecessary try/catch/finally block.
---
 lib/domain.js                                 | 33 ++++++++++---------
 .../parallel/test-domain-ee-error-listener.js | 20 +++++++++++
 2 files changed, 37 insertions(+), 16 deletions(-)
 create mode 100644 test/parallel/test-domain-ee-error-listener.js

diff --git a/lib/domain.js b/lib/domain.js
index b68e642c490cc3..d00b30cd332bd2 100644
--- a/lib/domain.js
+++ b/lib/domain.js
@@ -399,32 +399,33 @@ EventEmitter.init = function() {
 const eventEmit = EventEmitter.prototype.emit;
 EventEmitter.prototype.emit = function emit(...args) {
   const domain = this.domain;
-  if (domain === null || domain === undefined || this === process) {
-    return Reflect.apply(eventEmit, this, args);
-  }
 
   const type = args[0];
-  // edge case: if there is a domain and an existing non error object is given,
-  // it should not be errorized
-  // see test/parallel/test-event-emitter-no-error-provided-to-error-event.js
-  if (type === 'error' && args.length > 1 && args[1] &&
-    !(args[1] instanceof Error)) {
-    domain.emit('error', args[1]);
-    return false;
-  }
+  const doError = type === 'error' &&
+                  this._events !== undefined &&
+                  this._events.error !== undefined;
 
-  domain.enter();
-  try {
+  if (doError || domain === null || domain === undefined || this === process) {
     return Reflect.apply(eventEmit, this, args);
-  } catch (er) {
+  }
+
+  if (type === 'error') {
+    const er = args.length > 1 && args[1] ?
+      args[1] : new errors.Error('ERR_UNHANDLED_ERROR');
+
     if (typeof er === 'object' && er !== null) {
       er.domainEmitter = this;
       er.domain = domain;
       er.domainThrown = false;
     }
+
     domain.emit('error', er);
     return false;
-  } finally {
-    domain.exit();
   }
+
+  domain.enter();
+  const ret = Reflect.apply(eventEmit, this, args);
+  domain.exit();
+
+  return ret;
 };
diff --git a/test/parallel/test-domain-ee-error-listener.js b/test/parallel/test-domain-ee-error-listener.js
new file mode 100644
index 00000000000000..69041c7523b142
--- /dev/null
+++ b/test/parallel/test-domain-ee-error-listener.js
@@ -0,0 +1,20 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const domain = require('domain').create();
+const EventEmitter = require('events');
+
+domain.on('error', common.mustNotCall());
+
+const ee = new EventEmitter();
+
+const plainObject = { justAn: 'object' };
+ee.once('error', common.mustCall((err) => {
+  assert.deepStrictEqual(err, plainObject);
+}));
+ee.emit('error', plainObject);
+
+const err = new Error('test error');
+ee.once('error', common.expectsError(err));
+ee.emit('error', err);

From d46e02092b7e8067f21cf90691f15c9f0323fb56 Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Sun, 10 Dec 2017 13:38:43 -0500
Subject: [PATCH 2/4] fixup: use listenerCount instead of _events

---
 lib/domain.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/domain.js b/lib/domain.js
index d00b30cd332bd2..cd67d7ca139641 100644
--- a/lib/domain.js
+++ b/lib/domain.js
@@ -402,8 +402,7 @@ EventEmitter.prototype.emit = function emit(...args) {
 
   const type = args[0];
   const doError = type === 'error' &&
-                  this._events !== undefined &&
-                  this._events.error !== undefined;
+                  this.listenerCount(type) > 0;
 
   if (doError || domain === null || domain === undefined || this === process) {
     return Reflect.apply(eventEmit, this, args);

From f8361f74e9342ab196abecc7564d7f9c70643572 Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Mon, 11 Dec 2017 11:28:08 -0500
Subject: [PATCH 3/4] fixup: rename doError variable

---
 lib/domain.js | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/domain.js b/lib/domain.js
index cd67d7ca139641..ec43aed89004df 100644
--- a/lib/domain.js
+++ b/lib/domain.js
@@ -401,10 +401,13 @@ EventEmitter.prototype.emit = function emit(...args) {
   const domain = this.domain;
 
   const type = args[0];
-  const doError = type === 'error' &&
-                  this.listenerCount(type) > 0;
+  const shouldEmitError = type === 'error' &&
+                          this.listenerCount(type) > 0;
 
-  if (doError || domain === null || domain === undefined || this === process) {
+  // Just call original `emit` if current EE instance has `error`
+  // handler, there's no active domain or this is process
+  if (shouldEmitError || domain === null || domain === undefined ||
+      this === process) {
     return Reflect.apply(eventEmit, this, args);
   }
 

From f401c32d8b9c37cc1fd28a68df528113ffd670a0 Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Tue, 12 Dec 2017 16:40:25 -0500
Subject: [PATCH 4/4] fixup: remove unnecessary null check

---
 lib/domain.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/domain.js b/lib/domain.js
index ec43aed89004df..9670a53629e16b 100644
--- a/lib/domain.js
+++ b/lib/domain.js
@@ -415,7 +415,7 @@ EventEmitter.prototype.emit = function emit(...args) {
     const er = args.length > 1 && args[1] ?
       args[1] : new errors.Error('ERR_UNHANDLED_ERROR');
 
-    if (typeof er === 'object' && er !== null) {
+    if (typeof er === 'object') {
       er.domainEmitter = this;
       er.domain = domain;
       er.domainThrown = false;