diff --git a/lib/circuit.js b/lib/circuit.js index 8ef1e5ec..f9f2368a 100644 --- a/lib/circuit.js +++ b/lib/circuit.js @@ -640,7 +640,11 @@ class CircuitBreaker extends EventEmitter { function handleError (error, circuit, timeout, args, latency, resolve, reject) { clearTimeout(timeout); - fail(circuit, error, args, latency); + + if (circuit.options.errorFilter(error)) { + circuit.emit('success', error, latency); + } else fail(circuit, error, args, latency); + const fb = fallback(circuit, error, args); if (fb) resolve(fb); else reject(error); @@ -663,8 +667,6 @@ function fallback (circuit, err, args) { } function fail (circuit, err, args, latency) { - if (circuit.options.errorFilter(err)) return; - /** * Emitted when the circuit breaker action fails * @event CircuitBreaker#failure diff --git a/test/common.js b/test/common.js index a9310487..0bba12f2 100644 --- a/test/common.js +++ b/test/common.js @@ -58,6 +58,12 @@ function failedCallbackFunction () { Array.prototype.slice.call(arguments).pop()('Whoops!'); } +function failWithCode (errorCode) { + const err = new Error(`Failed with ${errorCode} status code`); + err.statusCode = errorCode; + return Promise.reject(err); +} + function identity (_) { return _; } module.exports = exports = { @@ -68,5 +74,6 @@ module.exports = exports = { callbackFunction, failedCallbackFunction, nonPromise, - identity + identity, + failWithCode }; diff --git a/test/error-filter-test.js b/test/error-filter-test.js index 214957de..4df5d289 100644 --- a/test/error-filter-test.js +++ b/test/error-filter-test.js @@ -2,12 +2,7 @@ const test = require('tape'); const CircuitBreaker = require('../lib/circuit'); - -function mightFail (errorCode) { - const err = new Error('Something went wrong'); - err.statusCode = errorCode; - return Promise.reject(err); -} +const { failWithCode } = require('./common'); const options = { errorThresholdPercentage: 1, @@ -18,14 +13,15 @@ const options = { }; test('Bypasses failure stats if errorFilter returns true', t => { - t.plan(3); + t.plan(4); - const breaker = new CircuitBreaker(mightFail, options); + const breaker = new CircuitBreaker(failWithCode, options); breaker.fire(400) .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); t.end(); }); @@ -34,7 +30,7 @@ test('Bypasses failure stats if errorFilter returns true', t => { test('Increments failure stats if errorFilter returns false', t => { t.plan(3); - const breaker = new CircuitBreaker(mightFail, options); + const breaker = new CircuitBreaker(failWithCode, options); breaker.fire(500) .then(t.fail) .catch(err => { @@ -47,7 +43,7 @@ test('Increments failure stats if errorFilter returns false', t => { test('Increments failure stats if no filter is provided', t => { t.plan(3); - const breaker = new CircuitBreaker(mightFail, + const breaker = new CircuitBreaker(failWithCode, { errorThresholdPercentage: 1 }); breaker.fire(500) .then(t.fail) diff --git a/test/half-open-test.js b/test/half-open-test.js index 42edb2e2..dc7fc88f 100644 --- a/test/half-open-test.js +++ b/test/half-open-test.js @@ -2,7 +2,7 @@ const test = require('tape'); const CircuitBreaker = require('../lib/circuit'); -const { timedFailingFunction } = require('./common'); +const { timedFailingFunction, failWithCode } = require('./common'); test('When half-open, the circuit only allows one request through', t => { t.plan(10); @@ -46,3 +46,39 @@ test('When half-open, the circuit only allows one request through', t => { .then(t.end); }, options.resetTimeout * 1.5); }); + +test('When half-open, a filtered error should close the circuit', t => { + t.plan(7); + const options = { + errorThresholdPercentage: 1, + resetTimeout: 100, + errorFilter: err => err.statusCode < 500 + }; + + const breaker = new CircuitBreaker(failWithCode, options); + breaker.fire(500) + .then(t.fail) + .catch(e => t.equals(e.message, 'Failed with 500 status code')) + .then(() => { + t.ok(breaker.opened, 'should be open after initial fire'); + t.notOk(breaker.pendingClose, + 'should not be pending close after initial fire'); + + // Fire again after reset timeout. should be half open + setTimeout(() => { + t.ok(breaker.halfOpen, 'should be halfOpen after timeout'); + t.ok(breaker.pendingClose, 'should be pending close after timeout'); + breaker + .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 failing with filtered error'); + }) + .then(_ => breaker.shutdown()) + .then(t.end); + }, options.resetTimeout * 1.5); + }); +});