Permalink
Browse files

Forward unhandled exceptions to main event loop

While walking the list of resolve callbacks and calling them, if
something throws an exception, use process.nextTick() to rethrow
the exception from the main event loop.

The reason to catch such exceptions at all is because they're in
client-provided code, and so the fiber-futures library shouldn't
get blamed for them.

The correct destination for such exceptions is the uncaughtException
handler, and that's what rethrowing the exception fron nextTick()
achieves.

The previous behavior was to call process.exit() if such an exception
should happen. That's too severe, however; exceptions in a resolve
callback should be no more or less illegal than exceptions anywhere
else.

Note that in conjunction with Future.wait(), the resolve callback will
often be the local cb() function defined inside Future.wait() itself,
which calls fiber.run(), which propagates any exceptions that should
happen inside the body of the fiber itself: this means it is actually
completely normal for any exceptions that happen inside fiber client
code to land (through fiber.run() and wait()'s cb() routine) in these
callback processing routines' exception handlers.

I don't know whether it's preferable to log the exception at the point
we rethrow it or not. Mostly it will be unwelcome spew, but occasionally
it could be vitally helpful. Also, such logging causes the new test for
this change to fail, because tests in the fibers testsuite are supposed
to print nothing more than "pass" to pass. As a compromise for discussion
I've left the log statement present but commented out.
  • Loading branch information...
1 parent 875409f commit ddb5a8042b11045f1b12ce0eac6c99a2827858fc @metamatt metamatt committed Mar 21, 2013
Showing with 8 additions and 4 deletions.
  1. +8 −4 future.js
View
@@ -168,8 +168,10 @@ Future.prototype = {
ref[0](undefined, value);
}
} catch(ex) {
- console.log(String(ex.stack || ex.message || ex));
- process.exit(1);
+ // console.log('Resolve cb threw', String(ex.stack || ex.message || ex));
+ process.nextTick(function() {
+ throw(ex);
+ });
}
}
}
@@ -199,8 +201,10 @@ Future.prototype = {
ref[0](error);
}
} catch(ex) {
- console.log(ex.stack || ex);
- process.exit(1);
+ // console.log('Resolve cb threw', String(ex.stack || ex.message || ex));
+ process.nextTick(function() {
+ throw(ex);
+ });
}
}
}

0 comments on commit ddb5a80

Please sign in to comment.