Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix for 8.20.0 #896

Merged
merged 2 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions lib/circuit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
/**
Expand Down
44 changes: 43 additions & 1 deletion test/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand All @@ -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);
Expand Down
Loading