CC-Archived / promise-as3

Promises/A+ compliant implementation in ActionScript 3.0
167 stars 52 forks source link

Consider implementing done() to rethrow Errors for unhandled rejections. #14

Closed johnyanarella closed 10 years ago

johnyanarella commented 11 years ago

One of the pitfalls of interacting with Promise-based APIs is the tendency for important errors to be silently swallowed unless an explicit rejection handler is specified.

For example:

promise.then( function () {
  // logic in your callback throws an error and it is interpreted as a rejection.
  throw new Error('Boom!');
});

// The error is swallowed.

Q.js addresses this issue by introducing a done() method that terminates a chain of Promises, forcing subsequent rejections to be thrown as exceptions.

promise.then( function () {
  // logic in your callback throws an error and it is interpreted as a rejection.
  throw new Error('Boom!');
}).done();

// The error is thrown on the next tick of the event loop.

The done() method ensures that unhandled rejections are rethrown.

soywiz commented 11 years ago

+1

ThomasBurleson commented 10 years ago

In your sample above, the done() is used within a promise chain... so it returns a promise. But the source signature is

public function done():void
{
    resolver.then( null, scheduleRethrowError );
}

I think the code should be corrected to:

public function done() : Promise
{
    return resolver.then( null, scheduleRethrowError );
}
ThomasBurleson commented 10 years ago

Also, what happens if a developer does this ?

promise = doWork()
            .then( generateBoomError )
            .done()
            .then( makeAnotherAsync )
            .always( );

If done( ) terminates a promise chain, then perhaps subsequent then( ) assignments should throw a RTE ?

ThomasBurleson commented 10 years ago

And finally, must always( ) be used before done ?

return doWork()
        .then( generateBoomError )
        .then( makeAnotherAsync )
        .always( )
        .done();
johnyanarella commented 10 years ago

Sorry, my example above was incorrect. done() does not return a Promise.

done() terminates a Promise chain. No additional chaining can occur after it.

always() can occur anywhere in a promise chain. It effectively becomes a finally for the preceding Promise.

johnyanarella commented 10 years ago

I've updated the example above, and fixed the same as it appeared in the asdocs and README.md.

Thanks for pointing that out!