Bugfix: coalesce re-used empty cachekey when cache was not used. (#896)

Bugfix: emit fire when circuitbreaker is disabled.

Co-authored-by: Daan <>

lib/circuit.js

@@ -660,12 +660,11 @@ class CircuitBreaker extends EventEmitter {
         : Promise.resolve(result);
     }
 
-    // Need to create variable here to prevent extra calls if cache is disabled
-    let cacheKey = '';
+    // Generate cachekey only when cache and/or coalesce is enabled.
+    const cacheKey = this.options.cache || this.options.coalesce ? this.options.cacheGetKey.apply(this, rest) : '';
 
     // If cache is enabled, check if we have a cached value
     if (this.options.cache) {
-      cacheKey = this.options.cacheGetKey.apply(this, rest);
       const cached = this.options.cacheTransport.get(cacheKey);
       if (cached) {
         /**

test/cache.js

@@ -143,7 +143,7 @@ test('Using coalesce cache + regular cache.', t => {
       const stats = breaker.status.stats;
 
       t.equals(stats.cacheHits, 1, 'hits the cache');
-      t.equals(stats.coalesceCacheHits, 2, 'not further hits to coalesce cache, it is now expired');
+      t.equals(stats.coalesceCacheHits, 2, 'not further hits to coalesce cache, normal cache takes preference');
       t.equals(stats.successes, 1, 'success once');
       t.equals(arg, expected,
         `cache hits:misses ${stats.cacheHits}:${stats.cacheMisses}`);
@@ -152,6 +152,48 @@ test('Using coalesce cache + regular cache.', t => {
     .catch(t.fail);
 });
 
+test('Using coalesce cache only.', t => {
+  t.plan(10);
+
+  const options = {
+    cache: false,
+    coalesce: true,
+    coalesceTTL: 200
+  };
+
+  const breaker = new CircuitBreaker(passFail, options);
+
+  // fire breaker three times in rapid succession, expect execution once.
+  Promise.all([
+    breaker.fire(expected),
+    breaker.fire(expected),
+    breaker.fire(expected)
+  ]).then(results => {
+    const stats = breaker.status.stats;
+    t.equals(stats.cacheHits, 0, 'does not hit the cache');
+    t.equals(stats.coalesceCacheHits, 2, 'hits coalesce cache twice');
+    t.equals(stats.fires, 3, 'fired thrice');
+    t.equals(stats.successes, 1, 'success once');
+    t.equals(results.length, 3, 'executed 3');
+    t.deepEqual(results, [expected, expected, expected],
+      `cache coalesceCacheHits:coalesceCacheMisses` +
+      `${stats.coalesceCacheHits}:${stats.coalesceCacheMisses}`);
+  })
+    // Re-fire breaker, expect cache hit as cache takes preference.
+    .then(() => new Promise(resolve => setTimeout(resolve, 250)))
+    .then(() => breaker.fire(expected)).then(arg => {
+      const stats = breaker.status.stats;
+
+      t.equals(stats.cacheHits, 0, 'does not hit the cache');
+      t.equals(stats.coalesceCacheHits, 2, 'not further hits to coalesce cache, it is now expired');
+      t.equals(stats.successes, 2, 'success twice, no cache used');
+      t.equals(arg, expected,
+        `cache hits:misses ${stats.cacheHits}:${stats.cacheMisses}`);
+    })
+    .then(t.end)
+    .catch(t.fail);
+});
+
 test('No coalesce cache.', t => {
   t.plan(5);
   const breaker = new CircuitBreaker(passFail);