meteor / promise

ES6 Promise polyfill with Fiber support
MIT License
63 stars 15 forks source link

Print unhandled exceptions in fiber #2

Closed martijnwalraven closed 9 years ago

martijnwalraven commented 9 years ago

Currently, unhandled exceptions thrown from Promise.await are silently swallowed. So in the following code the exception exits the program ('after' is never reached), but doesn't print the exception the way it would if it were thrown synchronously:

async function testAsync() {
  throw new Error('from testAsync');
}

function doRunCommand(options) {
  console.log('before');
  Promise.await(testAsync());
  console.log('after');
}

Not sure what the best approach is, but one solution might be to add default exception handling to the fibers created in https://github.com/meteor/promise/blob/master/fiber_pool.js. We could do something similar to what Meteor.bindEnvironment does https://github.com/meteor/meteor/blob/devel/packages/meteor/dynamics_nodejs.js#L84-L124.

martijnwalraven commented 9 years ago

I guess this is to be expected from the way it works, but because code after a Promise.awaitcall executes in a different fiber, uncaught exceptions are not printed from that point onward. So the issue is not limited to exceptions thrown from within the async function passed to Promise.await, but it affects all code executed afterwards. This makes it really hard to diagnose errors.

Here, 'from doRunComman' is never printed, but the process does exit ('after throw' is never reached).

async function testAsync() {
}

function doRunCommand(options) {
  console.log('before');
  Promise.await(testAsync());
  console.log('after');

  throw new Error('from doRunCommand');
  console.log('after throw');
}
stubailo commented 9 years ago

I also encountered the same issue when working with Knex transactions and this Promise implementation.

benjamn commented 9 years ago

I believe the crux of the problem is that I call fiber.run() to resume the Fiber from within a Promise callback, so any code executed after the Promise.await (but before the next Fiber.yield()) is effectively enclosed by that Promise. If an exception is thrown, that Promise will be rejected, but no one is listening.

martijnwalraven commented 9 years ago

Thanks for fixing these cases! I'm still having trouble with a different situation though. If I don't Promise.await an async function, an uncaught exception thrown from within that function is still not printed. Am I missing something? Is that expected behavior?

async function testAsync() {
  console.log('before in testAsync');
  throw new Error('from testAsync');
  console.log('after in testAsync');
}

function doRunCommand(options) {
  console.log('before');
  testAsync();
  console.log('after');
}

(I'm sorry about these primitive reproductions. Assume the code after the testAsync() call continues as usual, so the process doesn't exit.)

martijnwalraven commented 9 years ago

The solution turned out to be to use testAsync().done(). Thanks @benjamn!