promises-aplus / promises-spec

An open standard for sound, interoperable JavaScript promises—by implementers, for implementers.
https://promisesaplus.com/
Creative Commons Zero v1.0 Universal
1.84k stars 171 forks source link

UnhandledPromiseRejectionWarning #259

Closed kswope closed 6 years ago

kswope commented 6 years ago

This works as expected

   let p = Promise.reject( 'hi' )                                                                                                                      
   p.catch( ( x ) => { console.log( x ) } ) 

This also works as expected, but in native promises (node) and bluebird it also prints out an unhandled promise warning

   let p = Promise.reject( 'hi' )                                                                                                                      
   p.then( () => {} )                                                                                                                                  
   p.catch( ( x ) => { console.log( x ) } )

I found this by screwing up, I was supposed to have chained then and catch, but now I'm wondering if this unexpected warning is because of a hole in the spec. I'm not even sure anymore if the warnings or the behavior is wrong.

domenic commented 6 years ago

This isn't really an appropriate repository for support questions like this. I'd try StackOverflow.

briancavalier commented 6 years ago

@kswope The spec is correct, and your second example is working as intended (although outside the scope of this spec). In the example, there is an unobserved rejected promise. Keeping in mind that p.then (and p.catch) doesn't modify the fulfill/reject state of p, but rather returns a new promise:

let p = Promise.reject( 'hi' )

// this line creates a new promise that is also rejected because p is rejected and
// the second argument to then() has been omitted.  The newly created, rejected
// promise is never assigned, and so *can never be handled*.  Thus, runtimes correctly
// report the unhandled rejection.
p.then( () => {} )

// This line creates a new promise that will always fulfill.  Because it handles p's
// error, runtimes use this as an indication that p's rejection is now handled.  This
// has no bearing on the rejected promise created by the line above.
p.catch( ( x ) => { console.log( x ) } )

Hope that helps.

kswope commented 6 years ago

Yes, thanks for the help. The problem I had was the 'forking' nature of promises and the fact that NOBODY mentions this.

Would you believe the next day somebody posted an elaborate slideshow on promises and made the exact same mistake!

I did some testing and it seems that only found bluebird and native promises will log a warning. I can imagine how much code out there using promises makes this same mistake. Its telling that warning are now issued from the two most popular libraries.