Make unhandled Promise errors throw an exception

A promise will throw an exception unless an error handler is attached in the
same "tick" that the error is emitted. This is to avoid silent promise
failures.

diff --git a/doc/api.txt b/doc/api.txt
index 688bf680477e7967bcead6bafed1fc8a848eaa9f..e72fbe51ce10ca8958bf43c9c1c58e334e9be648 100644 (file)
--- a/doc/api.txt
+++ b/doc/api.txt
@@ -304,7 +304,47 @@ If you created the promise (by doing +new events.Promise()+) then call
 the moment due to a bug; use +emitSuccess+ instead.)
 
 +promise.emitError(arg1, arg2, ...)+ ::
-Emits the +"error"+ event.
+Emits the +"error"+ event. If a no error handler is attached to the promise
+between +promise.emitError()+ and +process.nextTick()+, an exception is
+thrown.
++
+To explain the exception behavior, assume you have a "computeQuestion"
+function as follows:
++
+----------------------------------------
+var events = require('events');
+function computeQuestion(answer) {
+  var promise = new events.Promise();
+  if (answer !== 42) {
+    promise.emitError('wrong answer');
+    return promise;
+  }
+  // compute the question for 42
+
+  return promise;
+}
+----------------------------------------
++
+You can stop an exception to be thrown here by attaching an errback handler
+right away (in the same event loop tick) like this:
++
+----------------------------------------
+computeQuestion(23).addErrback(function() {
+  // No exception will be thrown
+});
+----------------------------------------
++
+However, if you try to  attach the error handler in a later tick, the promise
+will already have thrown an exception:
++
+----------------------------------------
+var promise = computeQuestion(23);
+setTimeout(function() {
+  promise.addErrback(function() {
+    // This will never execute, the promise already threw an exception
+  });
+}, 1000);
+----------------------------------------
 
 +promise.timeout(timeout = undefined)+ ::
 If the +timeout+ parameter is provided, the promise will emit an +"error"+

diff --git a/src/node.js b/src/node.js
index 6fc0ea4402bbbdff2cdf769ecdd7f1c9ec02d52e..725291c44255d8a5615c09d16fc4f9c656ede920 100644 (file)
--- a/src/node.js
+++ b/src/node.js
@@ -289,24 +289,31 @@ var eventsModule = createInternalModule('events', function (exports) {
 
     this._values = Array.prototype.slice.call(arguments);
     this.emit.apply(this, ['error'].concat(this._values));
+
+    if (this.listeners('error').length == 0) {
+      var self = this;
+      process.nextTick(function() {
+        if (self.listeners('error').length == 0) {
+          throw new Error('Unhandled emitError: '+JSON.stringify(self._values));
+        }
+      });
+    }
   };
 
   exports.Promise.prototype.addCallback = function (listener) {
-    if (!this.hasFired) {
-      return this.addListener("success", listener);
+    if (this.hasFired) {
+      return listener.apply(this, this._values);
     }
 
-    listener.apply(this, this._values);
-    return this;
+    return this.addListener("success", listener);
   };
 
   exports.Promise.prototype.addErrback = function (listener) {
-    if (!this.hasFired) {
-      return this.addListener("error", listener);
+    if (this.hasFired) {
+      listener.apply(this, this._values);
     }
 
-    listener.apply(this, this._values);
-    return this;
+    return this.addListener("error", listener);
   };
 
   /* Poor Man's coroutines */
@@ -1010,10 +1017,6 @@ process.mainModule = createModule(".");
 var loadPromise = new events.Promise();
 process.mainModule.load(process.ARGV[1], loadPromise);
 
-loadPromise.addErrback(function(e) {
-  throw e;
-});
-
 // All our arguments are loaded. We've evaluated all of the scripts. We
 // might even have created TCP servers. Now we enter the main eventloop. If
 // there are no watchers on the loop (except for the ones that were

diff --git a/test/mjsunit/test-promise.js b/test/mjsunit/test-promise.js
index e042030c4f38826f363bfb852c082fab307f7510..9b314ea56e7a01a26491d35b43441b1429952e76 100644 (file)
--- a/test/mjsunit/test-promise.js
+++ b/test/mjsunit/test-promise.js
@@ -9,6 +9,8 @@ var
     a2: 1,
     b1: 1,
     b2: 1,
+    c1: 1,
+    d1: 1,
   };
 
 // Test regular & late callback binding
@@ -37,6 +39,27 @@ b.addErrback(function(value) {
   expectedCallbacks.b2--;
 });
 
+// Test late errback binding
+var c = new Promise();
+c.emitError(TEST_VALUE);
+c.addErrback(function(value) {
+  assert.equal(TEST_VALUE, value);
+  expectedCallbacks.c1--;
+});
+
+// Test errback exceptions
+var d = new Promise();
+d.emitError(TEST_VALUE);
+
+process.addListener('uncaughtException', function(e) {
+  if (e.name === "AssertionError") {
+    throw e;
+  }
+
+  expectedCallbacks.d1--;
+  assert.ok(e.message.match(/unhandled emitError/i));
+});
+
 process.addListener('exit', function() {
   for (var name in expectedCallbacks) {
     var count = expectedCallbacks[name];