From 7f488f132610e0a95cf64135f295b382bfe2cad2 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Mon, 28 Oct 2019 18:28:17 +0000 Subject: [PATCH] fix: Clear reset timer on open() (#383) * add failing test * clear reset timeout on open() * clear warmum timeout on shutdown() --- lib/circuit.js | 19 ++++++++++++++++--- test/open-test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 test/open-test.js diff --git a/lib/circuit.js b/lib/circuit.js index d6d1951b..d8011eda 100644 --- a/lib/circuit.js +++ b/lib/circuit.js @@ -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`; @@ -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(); } @@ -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); @@ -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; @@ -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; /** diff --git a/test/open-test.js b/test/open-test.js new file mode 100644 index 00000000..5ac233a7 --- /dev/null +++ b/test/open-test.js @@ -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); +});