From 7b9260227470ba11ef5998db19fbb9de39399536 Mon Sep 17 00:00:00 2001
From: Tom Jenkinson <tjenkinson@users.noreply.github.com>
Date: Sat, 26 Oct 2019 16:40:49 +0100
Subject: [PATCH] fix: do not close if preexisting task resolves when state is
 not OPEN (#382)

The breaker would close if an earlier task resolved after a later one closed it.
This fixes that and adds a test case.
---
 lib/circuit.js      |  6 ++++-
 test/closed-test.js | 54 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 test/closed-test.js

diff --git a/lib/circuit.js b/lib/circuit.js
index b80377e8..d6d1951b 100644
--- a/lib/circuit.js
+++ b/lib/circuit.js
@@ -187,7 +187,11 @@ class CircuitBreaker extends EventEmitter {
     }
 
     this.on('open', _startTimer(this));
-    this.on('success', _ => this.close());
+    this.on('success', _ => {
+      if (this.halfOpen) {
+        this.close();
+      }
+    });
     if (this.options.cache) {
       CACHE.set(this, undefined);
     }
diff --git a/test/closed-test.js b/test/closed-test.js
new file mode 100644
index 00000000..80379b37
--- /dev/null
+++ b/test/closed-test.js
@@ -0,0 +1,54 @@
+'use strict';
+
+const test = require('tape');
+const CircuitBreaker = require('../');
+
+test(
+  'When closed, an existing request resolving does not reopen circuit',
+  t => {
+    t.plan(6);
+    const options = {
+      errorThresholdPercentage: 1,
+      resetTimeout: 60000
+    };
+
+    const breaker = new CircuitBreaker(function (shouldPass, time) {
+      return new Promise((resolve, reject) => {
+        const timer = setTimeout(
+          shouldPass
+            ? () => resolve('success')
+            : () => reject(new Error('test-error')),
+          time || 0
+        );
+        if (typeof timer.unref === 'function') {
+          timer.unref();
+        }
+      });
+    }, options);
+
+    const promises = [];
+
+    promises.push(breaker.fire(true, 100)
+      .then(res => t.equals(res, 'success')));
+
+    promises.push(breaker.fire(false)
+      .then(t.fail)
+      .catch(e => {
+        t.equals(e.message, 'test-error');
+      })
+      .then(() => {
+        t.ok(breaker.opened, 'should be open after initial fire');
+        t.notOk(breaker.pendingClose,
+          'should not be pending close after initial fire');
+      }));
+
+    Promise.all(promises)
+      .then(() => {
+        t.ok(breaker.opened,
+          'should still be open even after first fire resolved');
+        t.notOk(breaker.pendingClose,
+          'should still not be pending close');
+      })
+      .then(() => t.end());
+  }
+);