medikoo / memoizee

Complete memoize/cache solution for JavaScript
ISC License
1.73k stars 61 forks source link

Unhandled rejection; breaking change #87

Closed gajus closed 7 years ago

gajus commented 7 years ago

Whatever change has been made in the last 7 days is causing my code to throw unhandled rejection. Assuming this is not a bug/ regression, this should have been a major version release.

gajus commented 7 years ago

It appears that setting promise: 'then' returns the original behaviour.

medikoo commented 7 years ago

@gajus can you provide more details? memoizee on it's own doesn't throw on unhandled rejections.

There was a change to promise handling, but it was more in a category of bug fix, than new feature. It's in older version where memoizee by default, assuming promise implementation provided done, thrown on rejections (and not necessary unhandled), it's not the case anymore.

medikoo commented 7 years ago

@gajus I looked closer and indeed there's a bug in promise handling, fix will be coming shortly.

medikoo commented 7 years ago

Fixed with https://github.com/medikoo/memoizee/commit/5b79698ef92dc3931719c4acff13f3998abb9184 and published as v0.4.10

Problem is that then:finally mode (which was turned on as default in case of promise implementation implenenting finally) stood on flawed assumption, that following won't report unhandled rejection:

var promise = Promise.reject(new Error("Rejection"));

promise.then(function () {});
promise.catch(function () {});

Thing is that promise created by promise.then also comes as rejection and that one will be reported as unhandled. I removed then:finally mode totally, as it can't work right.

Sorry for issues it caused :)