feat: Applying code review

nodeshift · Mar 9, 2017 · 6a0f7ff · 6a0f7ff
1 parent 6c3144f
commit 6a0f7ff
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 11 deletions.
diff --git a/lib/circuit.js b/lib/circuit.js
@@ -180,11 +180,21 @@ class CircuitBreaker extends EventEmitter {
    */
   fire () {
     if (CACHE.get(this) !== undefined) {
-      this.emit('cacheHit');
+      /**
+       * Emitted when the circuit breaker is using the cache
+       * @event CircuitBreaker#cacheHits
+       */
+      this.emit('cacheHits');
       return CACHE.get(this);
-    } else {
-      this.emit('cacheMiss');
+    } else if (this.options.cache) {
+      /**
+       * Emitted when the circuit breaker is not using the cache but
+       * the cache option is enabled.
+       * @event CircuitBreaker#cacheHits
+       */
+      this.emit('cacheMisses');
     }
+
     const args = Array.prototype.slice.call(arguments);
     /**
      * Emitted when the circuit breaker action is executed
@@ -205,7 +215,7 @@ class CircuitBreaker extends EventEmitter {
 
     let timeout;
     let timeoutError = false;
-    const value = new this.Promise((resolve, reject) => {
+    return new this.Promise((resolve, reject) => {
       timeout = setTimeout(
         () => {
           timeoutError = true;
@@ -233,6 +243,9 @@ class CircuitBreaker extends EventEmitter {
                */
               this.emit('success', result);
               resolve(result);
+              if (this.options.cache) {
+                CACHE.set(this, promise);
+              }
               clearTimeout(timeout);
             }
           })
@@ -242,10 +255,6 @@ class CircuitBreaker extends EventEmitter {
         handleError(error, this, timeout, args, resolve, reject);
       }
     });
-    if (this.options.cache) {
-      CACHE.set(this, value);
-    }
-    return value;
   }
 
   /**

diff --git a/test/test.js b/test/test.js
@@ -68,19 +68,19 @@ test('Using cache', (t) => {
   const breaker = cb(passFail, options);
 
   breaker.fire(expected)
-    .then((arg) => t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHit}:${breaker.status.cacheMiss}`))
+    .then((arg) => t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHits}:${breaker.status.cacheMisses}`))
     .catch(t.fail)
     .then(() => {
       breaker.fire(expected)
         .then((arg) => {
-          t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHit}:${breaker.status.cacheMiss}`);
+          t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHits}:${breaker.status.cacheMisses}`);
           breaker.clearCache();
         })
         .catch(t.fail)
         .then(() => {
           breaker.fire(expected)
             .then((arg) => {
-              t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHit}:${breaker.status.cacheMiss}`);
+              t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHits}:${breaker.status.cacheMisses}`);
             })
             .then(t.end)
             .catch(t.fail);
@@ -549,6 +549,23 @@ test('CircuitBreaker fallback as a CircuitBreaker that fails', (t) => {
     .catch((e) => t.equals(e, 'Error: -1 is < 0', 'Breaker should fail'))
     .then(t.end);
 });
+
+test('CircuitBreaker fallback as a CircuitBreaker', (t) => {
+  t.plan(1);
+  const options = {
+    maxFailures: 1,
+    resetTimeout: 100
+  };
+
+  const input = -1;
+  const breaker = cb(passFail, options);
+  breaker.fallback(cb((x) => x, options));
+
+  breaker.fire(input)
+    .then((v) => t.equals(v, input, 'Fallback value equals input'))
+    .then(t.end);
+});
+
 /**
  * Returns a promise that resolves if the parameter
  * 'x' evaluates to >= 0. Otherwise the returned promise fails.