Skip to content

Commit

Permalink
fix: Clear reset timer on open() (#383)
Browse files Browse the repository at this point in the history
* add failing test

* clear reset timeout on open()

* clear warmum timeout on shutdown()
  • Loading branch information
tjenkinson authored and lholmquist committed Oct 28, 2019
1 parent 7b92602 commit 7f488f1
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
19 changes: 16 additions & 3 deletions lib/circuit.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const ENABLED = Symbol('Enabled');
const WARMING_UP = Symbol('warming-up');
const VOLUME_THRESHOLD = Symbol('volume-threshold');
const OUR_ERROR = Symbol('our-error');
const RESET_TIMEOUT = Symbol('reset-timeout');
const WARMUP_TIMEOUT = Symbol('warmup-timeout');
const deprecation = `options.maxFailures is deprecated. \
Please use options.errorThresholdPercentage`;

Expand Down Expand Up @@ -136,8 +138,10 @@ class CircuitBreaker extends EventEmitter {
this[ENABLED] = options.enabled !== false;

if (this[WARMING_UP]) {
const timer = setTimeout(_ => (this[WARMING_UP] = false),
this.options.rollingCountTimeout);
const timer = this[WARMUP_TIMEOUT] = setTimeout(
_ => (this[WARMING_UP] = false),
this.options.rollingCountTimeout
);
if (typeof timer.unref === 'function') {
timer.unref();
}
Expand Down Expand Up @@ -175,7 +179,7 @@ class CircuitBreaker extends EventEmitter {

function _startTimer (circuit) {
return _ => {
const timer = setTimeout(() => {
const timer = circuit[RESET_TIMEOUT] = setTimeout(() => {
circuit[STATE] = HALF_OPEN;
circuit[PENDING_CLOSE] = true;
circuit.emit('halfOpen', circuit.options.resetTimeout);
Expand Down Expand Up @@ -222,6 +226,9 @@ class CircuitBreaker extends EventEmitter {
* @returns {void}
*/
open () {
if (this[RESET_TIMEOUT]) {
clearTimeout(this[RESET_TIMEOUT]);
}
this[PENDING_CLOSE] = false;
if (this[STATE] !== OPEN) {
this[STATE] = OPEN;
Expand All @@ -242,6 +249,12 @@ class CircuitBreaker extends EventEmitter {
shutdown () {
this.disable();
this.removeAllListeners();
if (this[RESET_TIMEOUT]) {
clearTimeout(this[RESET_TIMEOUT]);
}
if (this[WARMUP_TIMEOUT]) {
clearTimeout(this[WARMUP_TIMEOUT]);
}
this.status.shutdown();
this[STATE] = SHUTDOWN;
/**
Expand Down
31 changes: 31 additions & 0 deletions test/open-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

const test = require('tape');
const CircuitBreaker = require('../');
const { timedFailingFunction } = require('./common');

test('when open() is called it cancels the resetTimeout', t => {
t.plan(5);
const options = {
errorThresholdPercentage: 1,
resetTimeout: 100
};

const breaker = new CircuitBreaker(timedFailingFunction, options);
breaker.fire(0)
.then(t.fail)
.catch(e => t.equals(e, 'Failed after 0'))
.then(() => {
t.ok(breaker.opened, 'should be open after initial fire');
t.notOk(breaker.pendingClose,
'should not be pending close after initial fire');
breaker.open();
});

setTimeout(() => {
t.ok(breaker.opened, 'should not have been opened by the resetTimeout');
t.notOk(breaker.pendingClose,
'should still not be pending close');
t.end();
}, options.resetTimeout * 1.5);
});

0 comments on commit 7f488f1

Please sign in to comment.