From 69ce299dfc3e5454226f696a21db7cf5bbd677e4 Mon Sep 17 00:00:00 2001 From: Mariusz Nowak Date: Wed, 13 Jul 2016 09:46:22 +0200 Subject: [PATCH] Fix `promise` mode. To not use `then` & `finally` Additionally `done` mode was provided, so `done` can be used with no `finally` --- README.md | 8 ++++++-- ext/promise.js | 39 ++++++++++++++------------------------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 67526fd..6dd4f8b 100644 --- a/README.md +++ b/README.md @@ -152,9 +152,13 @@ memoized(3, 7); // Cache hit To avoid error swallowing and registration of error handlers, `done` and `finally` (if implemented) are preferred over `then` -Still relying on `done` may cause trouble if implementation that's used throws rejection reasons when `done` is called with no _onFail_ callback on rejected promise, even though error handler might have been registered through other `then` or `done` call. +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, please force usage of then by passing `'then'` string to `promise` option as: +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_ ```javascript memoized = memoize(afn, { promise: 'then' }); diff --git a/ext/promise.js b/ext/promise.js index cb484be..195dca5 100644 --- a/ext/promise.js +++ b/ext/promise.js @@ -11,13 +11,6 @@ var objectMap = require('es5-ext/object/map') require('../lib/registered-extensions').promise = function (mode, conf) { var waiting = create(null), cache = create(null), promises = create(null); - // We may want to force 'then' usage, when promise implementation that's used: - // - Implements both `done` and `finally` - // - For rejected promise throws rejection reason unconditionally when `done` is called with - // no onFailue callback (even though some other `then` or `done` call may have processed the - // error) - var forceThenMode = (mode === 'then'); - // After not from cache call conf.on('set', function (id, ignore, promise) { if (!isPromise(promise)) { @@ -42,31 +35,27 @@ require('../lib/registered-extensions').promise = function (mode, conf) { conf.delete(id); }; - // Use 'finally' and not rejection callback (on 'done' or 'then') to not register error - // handling. - // Additionally usage of 'finally' should take place if implementation - // supports promise cancelation (then no `then` or `done` callbacks are invoked) - var hasFinally = (typeof promise.finally === 'function'); - if (!forceThenMode && (typeof promise.done === 'function')) { + if ((mode !== 'then') && (typeof promise.done === 'function')) { // Optimal promise resolution - if (hasFinally) { + 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 { - // Be sure to escape error swallowing - if (hasFinally) { - promise.then(function (result) { nextTick(onSuccess.bind(this, result)); }); - promise.finally(function () { nextTick(onFailure); }); - } else { - promise.then(function (result) { - nextTick(onSuccess.bind(this, result)); - }, function () { - nextTick(onFailure); - }); - } + // With no `done` 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)); + }, function () { + nextTick(onFailure); + }); } });