feat: improve 'promise' mode handling

Improve default handling (fallback to 'then:finally' instead of 'done'). Support four different modes
This commit is contained in:
Mariusz Nowak
2017-08-28 17:31:44 +02:00
parent a257d6b9b7
commit 759e3157dd
2 changed files with 62 additions and 30 deletions

View File

@@ -150,20 +150,31 @@ memoized(3, 7); // Cache hit
###### Important notice on internal promises handling
To avoid error swallowing and registration of error handlers, `done` and `finally` (if implemented) are preferred over `then`
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.
Still relying on `done` & `finally` pair, may cause trouble if implementation that's used throws rejection reasons when `done` is called with no _onFail_ callback, even though error handler might have been registered through other `then` or `done` call.
If that's the case for you, you can force to not use `finally` or `done` (even if implemented) by providing following value to `promise` option:
- `'done'` - If `done` is implemented, it will purely try use `done` to register internal callbacks and not `finally` (even if it's implemented). If `done` is not implemented, this setting has no effect and callbacks are registered via `then`.
_This mode comes with side effect of silencing eventual 'Unhandled errors' on returned promise_
- `'then'` - No matter if `done` and `finally` are implemented, internal callbacks will be registered via `then`.
_This mode comes with side effect of silencing eventual 'Unhandled errors' on returned promise_
Alternatively we can force specific mode, by stating with `promise` option desired mode:
```javascript
memoized = memoize(afn, { promise: 'then' });
```
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
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
if no _onFailure_ callback was passed to `done`, and promise error was handled by other consumer (this is not commonly implemented _done_ behavior). Otherwise side-effect is that exception is thrown on promise rejection (highly not recommended)
##### Node.js callback style functions
With _async_ option we indicate that we memoize asynchronous (Node.js style) function

View File

@@ -2,18 +2,31 @@
"use strict";
var objectMap = require("es5-ext/object/map")
, isPromise = require("is-promise")
, nextTick = require("next-tick");
var objectMap = require("es5-ext/object/map")
, primitiveSet = require("es5-ext/object/primitive-set")
, ensureString = require("es5-ext/object/validate-stringifiable-value")
, toShortString = require("es5-ext/to-short-string-representation")
, isPromise = require("is-promise")
, nextTick = require("next-tick");
var create = Object.create;
var create = Object.create
, supportedModes = primitiveSet("then", "then:finally", "done", "done:finally");
require("../lib/registered-extensions").promise = function (mode, conf) {
var waiting = create(null), cache = create(null), promises = create(null);
if (mode === true) {
mode = null;
} else {
mode = ensureString(mode);
if (!supportedModes[mode]) {
throw new TypeError("'" + toShortString(mode) + "' is not valid promise mode");
}
}
// After not from cache call
conf.on("set", function (id, ignore, promise) {
var isFailed = false;
var isFailed = false, isSettled = false;
if (!isPromise(promise)) {
// Non promise result
@@ -46,23 +59,14 @@ require("../lib/registered-extensions").promise = function (mode, conf) {
conf.delete(id);
};
if (mode !== "then" && typeof promise.done === "function") {
// Optimal promise resolution
if (mode !== "done" && typeof promise.finally === "function") {
// Use 'finally' to not register error handling (still proper behavior is subject to
// used implementation, if library throws unconditionally even on handled errors
// switch to 'then' mode)
promise.done(onSuccess);
promise.finally(onFailure);
} else {
// With no `finally` side effect is that it mutes any eventual
// "Unhandled error" events on returned promise
promise.done(onSuccess, onFailure);
}
} else {
// With no `done` it's best we can do.
// Side effect is that it mutes any eventual "Unhandled error" events
// on returned promise
var resolvedMode = mode;
if (!resolvedMode) {
resolvedMode = typeof promise.finally === "function" ? "then:finally" : "then";
}
if (resolvedMode === "then") {
// With no `finally` it's best we can do, side effect is that it mutes any eventual
// "Unhandled error" events on returned promise
promise.then(
function (result) {
nextTick(onSuccess.bind(this, result));
@@ -71,6 +75,23 @@ require("../lib/registered-extensions").promise = function (mode, conf) {
nextTick(onFailure);
}
);
} else if (resolvedMode === "then:finally") {
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
promise.done(onSuccess, onFailure);
} else if (resolvedMode === "done:finally") {
// Cleanest solution assuming library does not throw unconditionally for rejected
// promises. Otherwise then:finally mode should be sued
promise.done(onSuccess);
promise.finally(onFailure);
}
});