fix: fix logic around pendingClose

nodeshift · Apr 24, 2017 · 4d89ae4 · 4d89ae4
1 parent 5e1171c
commit 4d89ae4
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 19 deletions.
diff --git a/lib/circuit.js b/lib/circuit.js
@@ -83,6 +83,7 @@ class CircuitBreaker extends EventEmitter {
       return _ => {
         const timer = setTimeout(() => {
           circuit[STATE] = HALF_OPEN;
+          circuit[PENDING_CLOSE] = true;
           circuit.emit('halfOpen', circuit.options.resetTimeout);
         }, circuit.options.resetTimeout);
         if (typeof timer.unref === 'function') {
@@ -92,7 +93,7 @@ class CircuitBreaker extends EventEmitter {
     }
 
     this.on('open', _startTimer(this));
-    this.on('success', () => this.close());
+    this.on('success', _ => this.close());
     if (this.options.cache) {
       CACHE.set(this, undefined);
     }
@@ -147,6 +148,10 @@ class CircuitBreaker extends EventEmitter {
     return this[GROUP];
   }
 
+  get pendingClose () {
+    return this[PENDING_CLOSE];
+  }
+
   /**
    * True if the circuit is currently closed. False otherwise.
    */
@@ -251,7 +256,7 @@ class CircuitBreaker extends EventEmitter {
       this.emit('cacheMiss');
     }
 
-    if (this.opened || (this.halfOpen && this[PENDING_CLOSE])) {
+    if (this.opened && !this.pendingClose) {
       /**
        * Emitted when the circuit breaker is open and failing fast
        * @event CircuitBreaker#reject
@@ -261,6 +266,7 @@ class CircuitBreaker extends EventEmitter {
       return fallback(this, 'Breaker is open', args) ||
         Promise.reject(new Error('Breaker is open'));
     }
+    this[PENDING_CLOSE] = false;
 
     let timeout;
     let timeoutError = false;
@@ -283,23 +289,22 @@ class CircuitBreaker extends EventEmitter {
           ? result
           : Promise.resolve(result);
 
-        promise
-          .then((result) => {
-            if (!timeoutError) {
-              /**
-               * Emitted when the circuit breaker action succeeds
-               * @event CircuitBreaker#success
-               */
-              this.emit('success', result);
-              resolve(result);
-              if (this.options.cache) {
-                CACHE.set(this, promise);
-              }
-              clearTimeout(timeout);
+        promise.then((result) => {
+          if (!timeoutError) {
+            clearTimeout(timeout);
+            /**
+             * Emitted when the circuit breaker action succeeds
+             * @event CircuitBreaker#success
+             */
+            this.emit('success', result);
+            resolve(result);
+            if (this.options.cache) {
+              CACHE.set(this, promise);
             }
-          })
-          .catch((error) =>
-            handleError(error, this, timeout, args, resolve, reject));
+          }
+        })
+        .catch((error) =>
+          handleError(error, this, timeout, args, resolve, reject));
       } catch (error) {
         handleError(error, this, timeout, args, resolve, reject);
       }

diff --git a/test/test.js b/test/test.js
@@ -523,7 +523,7 @@ test('CircuitBreaker events', (t) => {
 });
 
 test('circuit halfOpen', (t) => {
-  t.plan(8);
+  t.plan(14);
   const options = {
     errorThresholdPercentage: 1,
     resetTimeout: 100
@@ -534,24 +534,30 @@ test('circuit halfOpen', (t) => {
     .catch((e) => t.equals(e, 'Error: -1 is < 0', 'function should fail'))
     .then(() => {
       t.ok(breaker.opened, 'breaker should be open');
+      t.notOk(breaker.pendingClose, 'breaker should not be pending close');
     })
     .then(() => {
       setTimeout(() => {
         t.ok(breaker.halfOpen, 'breaker should be halfOpen');
+        t.ok(breaker.pendingClose, 'breaker should be pending close');
         // breaker should be half open, fail it again should open the circuit again
         breaker
           .fire(-1)
           .catch((e) => t.equals(e, 'Error: -1 is < 0', 'function should fail again'))
           .then(() => {
             t.ok(breaker.opened, 'breaker should be open again');
+            t.notOk(breaker.halfOpen, 'breaker should not be halfOpen');
+            t.notOk(breaker.pendingClose, 'breaker should not be pending close');
             setTimeout(() => {
               t.ok(breaker.halfOpen, 'breaker should be halfOpen again');
+              t.ok(breaker.pendingClose, 'breaker should be pending close');
               // breaker should be half open again and it should allow the original function to be called, and it should pass this time.
               breaker
                 .fire(1)
                 .then((result) => {
                   t.equals(1, result);
                   t.ok(breaker.closed, 'breaker should be closed');
+                  t.notOk(breaker.pendingClose, 'breaker should not be pending close');
                   t.end();
                 })
                 .catch(t.fail);