From 34f75a2994b3efd95fbd86fab5f6cf73f3fa39d8 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Mon, 7 Dec 2020 11:11:24 -0500 Subject: [PATCH] fix: catch exceptions in fallback functions (#510) When a fallback function is called and throws an exception, the circuit breaker should catch the exception and return a rejected Promise. See: https://github.com/nodeshift/opossum/issues/509 Signed-off-by: Lance Ball --- lib/circuit.js | 22 +++++++++++++--------- test/test.js | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lib/circuit.js b/lib/circuit.js index bde9c6db..40c06ae1 100644 --- a/lib/circuit.js +++ b/lib/circuit.js @@ -661,17 +661,21 @@ function handleError (error, circuit, timeout, args, latency, resolve, reject) { function fallback (circuit, err, args) { if (circuit[FALLBACK_FUNCTION]) { - const result = + try { + const result = circuit[FALLBACK_FUNCTION] .apply(circuit[FALLBACK_FUNCTION], [...args, err]); - /** - * Emitted when the circuit breaker executes a fallback function - * @event CircuitBreaker#fallback - * @type {any} the return value of the fallback function - */ - circuit.emit('fallback', result, err); - if (result instanceof Promise) return result; - return Promise.resolve(result); + /** + * Emitted when the circuit breaker executes a fallback function + * @event CircuitBreaker#fallback + * @type {any} the return value of the fallback function + */ + circuit.emit('fallback', result, err); + if (result instanceof Promise) return result; + return Promise.resolve(result); + } catch (e) { + return Promise.reject(e); + } } } diff --git a/test/test.js b/test/test.js index ff83df7a..70162a68 100644 --- a/test/test.js +++ b/test/test.js @@ -776,6 +776,22 @@ test('CircuitBreaker fallback as a CircuitBreaker', t => { .then(t.end); }); +test('CircuitBreaker fallback that throws returns a rejected Promise', t => { + t.plan(1); + const options = { + errorThresholdPercentage: 1, + resetTimeout: 100 + }; + const breaker = new CircuitBreaker(passFail, options); + breaker.fallback(_ => { throw new Error('Fallback failed'); }); + + breaker.fire(-1) + .then(_ => t.fail('CircuitBreaker fallback should return rejected promise')) + .catch(e => t.equals(e.message, 'Fallback failed')) + .then(_ => breaker.shutdown()) + .then(t.end); +}); + test('options.maxFailures should be deprecated', t => { const options = { maxFailures: 1 }; const originalLog = console.error;