From 6c8a42b19570ce30d8aa2c7b5a54a99ddf491569 Mon Sep 17 00:00:00 2001
From: Lucas Holmquist <lholmqui@redhat.com>
Date: Tue, 16 Mar 2021 18:30:26 -0400
Subject: [PATCH 1/2] fix!: A succesful erroFilter should not trigger the
 fallback function

---
 lib/circuit.js            | 13 ++++++++-----
 test/error-filter-test.js | 10 ++++++----
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/circuit.js b/lib/circuit.js
index 89a7c7dc..763dfa1b 100644
--- a/lib/circuit.js
+++ b/lib/circuit.js
@@ -655,15 +655,18 @@ class CircuitBreaker extends EventEmitter {
 function handleError (error, circuit, timeout, args, latency, resolve, reject) {
   clearTimeout(timeout);
 
-  if (circuit.options.errorFilter(error, ...args)) {
+  let errorFiltered;
+  if ((errorFiltered = circuit.options.errorFilter(error, ...args))) {
     circuit.emit('success', error, latency);
+    resolve(errorFiltered);
   } else {
     fail(circuit, error, args, latency);
-  }
 
-  const fb = fallback(circuit, error, args);
-  if (fb) resolve(fb);
-  else reject(error);
+    // Only call the fallback function if there errorFilter doesn't succeed
+    const fb = fallback(circuit, error, args);
+    if (fb) resolve(fb);
+    else reject(error);
+  }
 }
 
 function fallback (circuit, err, args) {
diff --git a/test/error-filter-test.js b/test/error-filter-test.js
index 5925eb79..5709a2b2 100644
--- a/test/error-filter-test.js
+++ b/test/error-filter-test.js
@@ -12,18 +12,20 @@ const options = {
   errorFilter: err => err.statusCode < 500
 };
 
-test('Bypasses failure stats if errorFilter returns true', t => {
+test('Bypasses failure stats if errorFilter returns true - Should return a success', t => {
   t.plan(4);
 
   const breaker = new CircuitBreaker(failWithCode, options);
   breaker.fire(400)
-    .then(t.fail)
-    .catch(err => {
-      t.equal(err.statusCode, 400);
+    .then(returnValue => {
+      t.equal(returnValue, true, 'return value is the value of the errorFilter function');
       t.equal(breaker.stats.failures, 0);
       t.equal(breaker.stats.successes, 1);
       t.ok(breaker.closed);
       t.end();
+    })
+    .catch(_ => {
+      t.fail();
     });
 });
 

From 6c58585ed9b3005cdd8bfe2d47257f0f65c9fafc Mon Sep 17 00:00:00 2001
From: Lucas Holmquist <lholmqui@redhat.com>
Date: Wed, 17 Mar 2021 14:08:23 -0400
Subject: [PATCH 2/2] squash: fix tests

---
 test/half-open-test.js | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/test/half-open-test.js b/test/half-open-test.js
index 8758fcd2..9fe594b1 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(7);
+  t.plan(6);
   const options = {
     errorThresholdPercentage: 1,
     resetTimeout: 100,
@@ -69,13 +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) // fail with a filtered error
-          .catch(e =>
-            t.equals(e.message, 'Failed with 400 status code',
-              'should fail again'))
+          .fire(400) // pass with a filtered error
           .then(() => {
             t.ok(breaker.closed,
-              'should be closed after failing with filtered error');
+              'should be closed after passing with filtered error');
           })
           .then(_ => breaker.shutdown())
           .then(t.end);