feat: enter halfOpen state on reinitialization if required (#720)

* feat: enter halfOpen state on reinitialization if required

When a circuit is reinitialized with state from a previous instance, if
the previous circuit was closed but the reset timeout has passed since
reinitialization, the circuit should enter halfOpen immediately.

Fixes: #719

Signed-off-by: Lance Ball <lball@redhat.com>

* fixup: use Object.prototype.hasOwnProperty.call()

Node.js 14 does not support Object.hasOwn(), so tests need to use the
older/deprecated(?) hasOwnProperty().

Signed-off-by: Lance Ball <lball@redhat.com>

Signed-off-by: Lance Ball <lball@redhat.com>

lib/circuit.js

@@ -21,6 +21,7 @@ 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 LAST_TIMER_AT = Symbol('last-timer-at');
 const deprecation = `options.maxFailures is deprecated. \
 Please use options.errorThresholdPercentage`;
 
@@ -190,7 +191,6 @@ class CircuitBreaker extends EventEmitter {
       // Open should be the opposite of closed,
       // but also the opposite of half_open
       this[OPEN] = !this[CLOSED] && !this[HALF_OPEN];
-
       this[SHUTDOWN] = options.state.shutdown || false;
     } else {
       this[PENDING_CLOSE] = false;
@@ -238,26 +238,37 @@ class CircuitBreaker extends EventEmitter {
      * @private
      */
     function _startTimer (circuit) {
+      circuit[LAST_TIMER_AT] = Date.now();
       return _ => {
         const timer = circuit[RESET_TIMEOUT] = setTimeout(() => {
-          circuit[STATE] = HALF_OPEN;
-          circuit[PENDING_CLOSE] = true;
-          /**
-           * Emitted after `options.resetTimeout` has elapsed, allowing for
-           * a single attempt to call the service again. If that attempt is
-           * successful, the circuit will be closed. Otherwise it remains open.
-           *
-           * @event CircuitBreaker#halfOpen
-           * @type {Number} how long the circuit remained open
-           */
-          circuit.emit('halfOpen', circuit.options.resetTimeout);
+          _halfOpen(circuit);
         }, circuit.options.resetTimeout);
         if (typeof timer.unref === 'function') {
           timer.unref();
         }
       };
     }
 
+    /**
+     * Sets the circuit breaker to half open
+     * @private
+     * @param {CircuitBreaker} circuit The current circuit breaker
+     * @returns {void}
+     */
+    function _halfOpen (circuit) {
+      circuit[STATE] = HALF_OPEN;
+      circuit[PENDING_CLOSE] = true;
+      /**
+       * Emitted after `options.resetTimeout` has elapsed, allowing for
+       * a single attempt to call the service again. If that attempt is
+       * successful, the circuit will be closed. Otherwise it remains open.
+       *
+       * @event CircuitBreaker#halfOpen
+       * @type {Number} how long the circuit remained open
+       */
+      circuit.emit('halfOpen', circuit.options.resetTimeout);
+    }
+
     this.on('open', _startTimer(this));
     this.on('success', _ => {
       if (this.halfOpen) {
@@ -275,9 +286,15 @@ class CircuitBreaker extends EventEmitter {
     } else if (this[CLOSED]) {
       this.close();
     } else if (this[OPEN]) {
-      // If the state being passed in is OPEN
-      // THen we need to start some timers
-      this.open();
+      // If the state being passed in is OPEN but more time has elapsed
+      // than the resetTimeout, then we should be in halfOpen state
+      if (this.options.state.lastTimerAt !== undefined &&
+        (Date.now() - this.options.state.lastTimerAt) >
+        this.options.resetTimeout) {
+        _halfOpen(this);
+      } else {
+        this.open();
+      }
     } else if (this[HALF_OPEN]) {
       // Not sure if anything needs to be done here
       this[STATE] = HALF_OPEN;
@@ -432,7 +449,8 @@ class CircuitBreaker extends EventEmitter {
         open: this.opened,
         halfOpen: this.halfOpen,
         warmUp: this.warmUp,
-        shutdown: this.isShutdown
+        shutdown: this.isShutdown,
+        lastTimerAt: this[LAST_TIMER_AT]
       },
       status: this.status.stats
     };

test/state-test.js

@@ -5,7 +5,7 @@ const CircuitBreaker = require('../');
 const { timedFailingFunction, passFail } = require('./common');
 
 test('CircuitBreaker State - export the state of a breaker instance', t => {
-  t.plan(7);
+  t.plan(8);
   const breaker = new CircuitBreaker(passFail);
 
   t.ok(breaker.toJSON, 'has the toJSON function');
@@ -17,6 +17,7 @@ test('CircuitBreaker State - export the state of a breaker instance', t => {
   t.equal(breakerState.state.halfOpen, false, 'half open initialized value');
   t.equal(breakerState.state.warmUp, false, 'warmup initialized value');
   t.equal(breakerState.state.shutdown, false, 'shutdown initialized value');
+  t.assert(Object.prototype.hasOwnProperty.call(breakerState.state, 'lastTimerAt'), 'lastTimerAt initialized value');
   t.end();
 });
 
@@ -80,8 +81,6 @@ test('Pre-populate state as Open(Closed === false) - Breaker resets after a conf
     }
   });
 
-  // Now the breaker should be open. Wait for reset and
-  // fire again.
   setTimeout(() => {
     breaker.fire(100)
       .catch(t.fail)
@@ -91,6 +90,26 @@ test('Pre-populate state as Open(Closed === false) - Breaker resets after a conf
   }, resetTimeout * 1.25);
 });
 
+test('Enters halfOpen state if closed is false and more time has elapsed since resetTimeout', t => {
+  t.plan(2);
+  const resetTimeout = 100;
+  const breaker = new CircuitBreaker(passFail, {
+    errorThresholdPercentage: 1,
+    resetTimeout,
+    state: {
+      closed: false,
+      lastTimerAt: Date.now() - (resetTimeout * 1.25)
+    }
+  });
+
+  t.ok(breaker.halfOpen, 'breaker should be halfOpen');
+  breaker.fire(100)
+    .catch(t.fail)
+    .then(arg => t.equals(arg, 100, 'breaker has reset'))
+    .then(_ => breaker.shutdown())
+    .then(t.end);
+});
+
 test('When half-open, the circuit only allows one request through', t => {
   t.plan(7);
   const options = {