Fix stuck halfOpen when using errorFilter

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

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
 };

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)

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);
+    });
+});