diff --git a/lib/circuit.js b/lib/circuit.js index bbbf3fb3..c94b950f 100644 --- a/lib/circuit.js +++ b/lib/circuit.js @@ -655,18 +655,20 @@ class CircuitBreaker extends EventEmitter { function handleError (error, circuit, timeout, args, latency, resolve, reject) { clearTimeout(timeout); - let errorFiltered; - if ((errorFiltered = circuit.options.errorFilter(error, ...args))) { + if (circuit.options.errorFilter(error, ...args)) { + // The error was filtered, so emit 'success' circuit.emit('success', error, latency); - resolve(errorFiltered); } else { + // Error was not filtered, so emit 'failure' fail(circuit, error, args, latency); - // Only call the fallback function if there errorFilter doesn't succeed + // Only call the fallback function if errorFilter doesn't succeed + // If the fallback function succeeds, resolve const fb = fallback(circuit, error, args); - if (fb) resolve(fb); - else reject(error); + if (fb) return resolve(fb); } + // In all other cases, reject + reject(error); } function fallback (circuit, err, args) { diff --git a/test/error-filter-test.js b/test/error-filter-test.js index 5709a2b2..48437c14 100644 --- a/test/error-filter-test.js +++ b/test/error-filter-test.js @@ -2,7 +2,7 @@ const test = require('tape'); const CircuitBreaker = require('../'); -const { failWithCode } = require('./common'); +const { failWithCode, passFail } = require('./common'); const options = { errorThresholdPercentage: 1, @@ -17,8 +17,9 @@ test('Bypasses failure stats if errorFilter returns true - Should return a succe const breaker = new CircuitBreaker(failWithCode, options); breaker.fire(400) - .then(returnValue => { - t.equal(returnValue, true, 'return value is the value of the errorFilter function'); + .then(t.fail) + .catch(err => { + t.equal(err.statusCode, 400); t.equal(breaker.stats.failures, 0); t.equal(breaker.stats.successes, 1); t.ok(breaker.closed); @@ -76,3 +77,32 @@ test('Provides invocation parameters to error filter', t => { t.end(); }); }); + +test('A successful errorFilter should not invoke the fallback function', t => { + t.plan(1); + const breaker = new CircuitBreaker(passFail, + { + errorThresholdPercentage: 1, + errorFilter: _ => { + t.ok(true, 'Error filter invoked'); + return true; + }, + fallback: _ => t.fail('Fallback function should not be called') + }); + breaker.fire(-1).catch(_ => t.end()); +}); + +test('A successful errorFilter should still return the error from function invocation', t => { + t.plan(2); + const breaker = new CircuitBreaker(passFail, + { + errorThresholdPercentage: 1, + errorFilter: _ => t.ok(true, 'Error filter invoked'), + fallback: _ => t.fail('Fallback function should not be called') + }); + breaker.fire(-1).catch(err => { + console.error(err); + t.equal(err, 'Error: -1 is < 0'); + t.end(); + }); +}); diff --git a/test/half-open-test.js b/test/half-open-test.js index 9fe594b1..c3616baf 100644 --- a/test/half-open-test.js +++ b/test/half-open-test.js @@ -48,7 +48,7 @@ test('When half-open, the circuit only allows one request through', t => { }); test('When half-open, a filtered error should close the circuit', t => { - t.plan(6); + t.plan(7); const options = { errorThresholdPercentage: 1, resetTimeout: 100, @@ -69,7 +69,10 @@ test('When half-open, a filtered error should close the circuit', t => { t.ok(breaker.halfOpen, 'should be halfOpen after timeout'); t.ok(breaker.pendingClose, 'should be pending close after timeout'); breaker - .fire(400) // pass with a filtered error + .fire(400) // fail with a filtered error + .catch(e => + t.equals(e.message, 'Failed with 400 status code', + 'should fail again')) .then(() => { t.ok(breaker.closed, 'should be closed after passing with filtered error');