fix: remove then:finally mode as it can't work right

In case of rejections it forces unhandledRejection warning, even if they're handled on promises returned to consumer
This commit is contained in:
Mariusz Nowak
2017-09-07 08:55:24 +02:00
parent c838ab3ab1
commit 5b79698ef9
2 changed files with 7 additions and 31 deletions

View File

@@ -150,25 +150,18 @@ memoized(3, 7); // Cache hit
###### Important notice on internal promises handling
To avoid error swallowing and registration of error handlers, `finally` (if implemented) is used internally
to detect eventual promise rejection. Otherwise default handling stands purely on _then_ which has side-effect
of muting eventual unhandled rejection notifications.
Alternatively we can force specific mode, by stating with `promise` option desired mode:
Default handling stands purely on _then_ which has side-effect of muting eventual unhandled rejection notifications.
Alternatively we can other (explained below), by stating with `promise` option desired mode:
```javascript
memoized = memoize(afn, { promise: 'then' });
memoized = memoize(afn, { promise: 'done:finally' });
```
Supported modes
- `then` _(default if promise does not implement `finally`)_. Values are resolved purely by
passing callbacks to `promise.then`. __Side effect is that eventual unhandled rejection on given promise
- `then` _(default)_. Values are resolved purely by passing callbacks to `promise.then`. __Side effect is that eventual unhandled rejection on given promise
come with no logged warning!__, and that to avoid implied error swallowing both states are resolved tick after callbacks were invoked
- `then:finally` _(default if promise does implement `finally`)_. Side effect is that to avoid implied error swallowing success value is processed tick after callbacks were invoked
- `done` Values are resolved purely by passing callback to `done` method. __Side effect is that eventual unhandled rejection on given promise come with now logged warning!__.
- `done:finally` The only method that may work with no side-effects assuming that promise implementaion does not throw unconditionally

View File

@@ -28,7 +28,7 @@ require("../lib/registered-extensions").promise = function (mode, conf) {
// After not from cache call
conf.on("set", function (id, ignore, promise) {
var isFailed = false, isSettled = false;
var isFailed = false;
if (!isPromise(promise)) {
// Non promise result
@@ -44,7 +44,7 @@ require("../lib/registered-extensions").promise = function (mode, conf) {
throw new Error(
"Memoizee error: Detected unordered then|done & finally resolution, which " +
"in turn makes proper detection of success/failure impossible (when in " +
"'then:finally' or 'done:finally' mode)\n" +
"'done:finally' mode)\n" +
"Consider to rely on 'then' or 'done' mode instead."
);
}
@@ -62,9 +62,7 @@ require("../lib/registered-extensions").promise = function (mode, conf) {
};
var resolvedMode = mode;
if (!resolvedMode) {
resolvedMode = typeof promise.finally === "function" ? "then:finally" : "then";
}
if (!resolvedMode) resolvedMode = "then";
if (resolvedMode === "then") {
// With no `finally` it's best we can do, side effect is that it mutes any eventual
@@ -77,21 +75,6 @@ require("../lib/registered-extensions").promise = function (mode, conf) {
nextTick(onFailure);
}
);
} else if (resolvedMode === "then:finally") {
if (typeof promise.finally !== "function") {
throw new Error(
"Memoizee error: Retrieved promise does not implement 'finally' " +
"in 'then:finally' mode"
);
}
promise.then(function (result) {
isSettled = true;
nextTick(onSuccess.bind(this, result));
});
promise.finally(function () {
if (isSettled) return;
onFailure();
});
} else if (resolvedMode === "done") {
// Not recommended, as it may mute any eventual "Unhandled error" events
if (typeof promise.done !== "function") {