Skip to content

Commit

Permalink
fix!: A succesful erroFilter should not trigger the fallback function.
Browse files Browse the repository at this point in the history
BREAKING CHANGE:  this is a semver major change

Previously, if an errorFilter  function passed it would emit success but still call the fallback function.  This corrects this behavior.  even a passing errorFilter is a success

fixes #540
  • Loading branch information
lholmquist committed Mar 18, 2021
1 parent 334283d commit 8a4fb7c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 15 deletions.
13 changes: 8 additions & 5 deletions lib/circuit.js
Expand Up @@ -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) {
Expand Down
10 changes: 6 additions & 4 deletions test/error-filter-test.js
Expand Up @@ -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();
});
});

Expand Down
9 changes: 3 additions & 6 deletions test/half-open-test.js
Expand Up @@ -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,
Expand All @@ -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);
Expand Down

0 comments on commit 8a4fb7c

Please sign in to comment.