From 6822136d8d4d11e94046cc364272d02c1d461702 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Thu, 17 Oct 2019 19:19:32 +0100 Subject: [PATCH 1/3] implement `isOurError()` Which provides a way of determining if the rejection from `fire()` or `call()` is from the circuit breaker or the action. --- lib/circuit.js | 30 ++++++++++++++++++++++++------ test/circuit-shutdown-test.js | 5 ++++- test/test.js | 27 +++++++++++++++++++++------ 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/lib/circuit.js b/lib/circuit.js index 337e79d3..645cc830 100644 --- a/lib/circuit.js +++ b/lib/circuit.js @@ -18,6 +18,7 @@ const CACHE = new WeakMap(); const ENABLED = Symbol('Enabled'); const WARMING_UP = Symbol('warming-up'); const VOLUME_THRESHOLD = Symbol('volume-threshold'); +const OUR_ERROR = Symbol('our-error'); const deprecation = `options.maxFailures is deprecated. \ Please use options.errorThresholdPercentage`; @@ -96,6 +97,16 @@ Please use options.errorThresholdPercentage`; * @fires CircuitBreaker#failure */ class CircuitBreaker extends EventEmitter { + /** + * Returns true if the provided error was generated here. It will be false + * if the error came from the action itself. + * @param {Error} error The Error to check. + * @returns {Boolean} true if the error was generated here + */ + static isOurError (error) { + return !!error[OUR_ERROR]; + } + constructor (action, options = {}) { super(); this.options = options; @@ -363,7 +374,8 @@ class CircuitBreaker extends EventEmitter { * function. * * @return {Promise} promise resolves with the circuit function's return - * value on success or is rejected on failure of the action. + * value on success or is rejected on failure of the action. Use isOurError() to + * determine if a rejection was a result of the circuit breaker or the action. * * @fires CircuitBreaker#failure * @fires CircuitBreaker#fallback @@ -402,7 +414,7 @@ class CircuitBreaker extends EventEmitter { */ call (context, ...rest) { if (this.isShutdown) { - const err = new Error('The circuit has been shutdown.'); + const err = tagError(new Error('The circuit has been shutdown.')); err.code = 'ESHUTDOWN'; return Promise.reject(err); } @@ -445,7 +457,7 @@ class CircuitBreaker extends EventEmitter { * @event CircuitBreaker#reject * @type {Error} */ - const error = new Error('Breaker is open'); + const error = tagError(new Error('Breaker is open')); error.code = 'EOPENBREAKER'; this.emit('reject', error); @@ -464,8 +476,9 @@ class CircuitBreaker extends EventEmitter { timeout = setTimeout( () => { timeoutError = true; - const error = - new Error(`Timed out after ${this.options.timeout}ms`); + const error = tagError( + new Error(`Timed out after ${this.options.timeout}ms`) + ); error.code = 'ETIMEDOUT'; /** * Emitted when the circuit breaker action takes longer than @@ -518,7 +531,7 @@ class CircuitBreaker extends EventEmitter { } } else { const latency = Date.now() - latencyStartTime; - const err = new Error('Semaphore locked'); + const err = tagError(new Error('Semaphore locked')); err.code = 'ESEMLOCKED'; /** * Emitted when the rate limit has been reached and there @@ -656,6 +669,11 @@ function fail (circuit, err, args, latency) { } } +function tagError (error) { + error[OUR_ERROR] = true; + return error; +} + // http://stackoverflow.com/a/2117523 const nextName = () => 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, c => { diff --git a/test/circuit-shutdown-test.js b/test/circuit-shutdown-test.js index d7d927b7..7d4f8d1a 100644 --- a/test/circuit-shutdown-test.js +++ b/test/circuit-shutdown-test.js @@ -17,7 +17,7 @@ test('EventEmitter max listeners', t => { }); test('Circuit shuts down properly', t => { - t.plan(5); + t.plan(6); const breaker = new CircuitBreaker(passFail); t.ok(breaker.fire(1), 'breaker is active'); breaker.shutdown(); @@ -28,6 +28,9 @@ test('Circuit shuts down properly', t => { .catch(err => { t.equals('ESHUTDOWN', err.code); t.equals('The circuit has been shutdown.', err.message); + t.equals( + CircuitBreaker.isOurError(err), true, 'isOurError() should return true' + ); t.end(); }); }); diff --git a/test/test.js b/test/test.js index f04aa6fe..6159db59 100644 --- a/test/test.js +++ b/test/test.js @@ -114,20 +114,23 @@ test('Using cache', t => { }); test('Fails when the circuit function fails', t => { - t.plan(1); + t.plan(2); const breaker = new CircuitBreaker(passFail); breaker.fire(-1) .then(() => t.fail) .catch(e => { t.equals(e, 'Error: -1 is < 0', 'expected error caught'); + t.equals( + CircuitBreaker.isOurError(e), false, 'isOurError() should return false' + ); }) .then(_ => breaker.shutdown()) .then(t.end); }); test('Fails when the circuit function times out', t => { - t.plan(2); + t.plan(3); const expected = 'Timed out after 10ms'; const expectedCode = 'ETIMEDOUT'; const breaker = new CircuitBreaker(slowFunction, { timeout: 10 }); @@ -137,6 +140,9 @@ test('Fails when the circuit function times out', t => { .catch(e => { t.equals(e.message, expected, 'timeout message received'); t.equals(e.code, expectedCode, 'ETIMEDOUT'); + t.equals( + CircuitBreaker.isOurError(e), true, 'isOurError() should return true' + ); }) .then(_ => breaker.shutdown()) .then(t.end); @@ -189,7 +195,7 @@ test('Works with callback functions that fail', t => { }); test('Breaker opens after a configurable number of failures', t => { - t.plan(2); + t.plan(3); const breaker = new CircuitBreaker(passFail, { errorThresholdPercentage: 10 }); @@ -201,7 +207,10 @@ test('Breaker opens after a configurable number of failures', t => { // with a valid value breaker.fire(100) .then(t.fail) - .catch(e => t.equals(e.message, 'Breaker is open', 'breaker opens')) + .catch(e => { + t.equals(e.message, 'Breaker is open', 'breaker opens'); + t.equals(CircuitBreaker.isOurError(e), true, 'isOurError() should return true'); + }) .then(_ => breaker.shutdown()) .then(t.end); }) @@ -282,12 +291,15 @@ test('Executes fallback action, if one exists, when breaker is open', t => { }); test('Passes error as last argument to the fallback function', t => { - t.plan(1); + t.plan(2); const fails = -1; const breaker = new CircuitBreaker(passFail, { errorThresholdPercentage: 1 }); breaker.on('fallback', result => { t.equals(result, `Error: ${fails} is < 0`, 'fallback received error as last parameter'); + t.equals( + CircuitBreaker.isOurError(result), false, 'isOurError() should return false' + ); breaker.shutdown(); t.end(); }); @@ -372,11 +384,14 @@ test('CircuitBreaker executes fallback when an action throws', t => { }); test('CircuitBreaker emits failure when falling back', t => { - t.plan(2); + t.plan(3); const breaker = new CircuitBreaker(passFail).fallback(() => 'fallback value'); breaker.on('failure', err => { t.equals('Error: -1 is < 0', err, 'Expected failure'); + t.equals( + CircuitBreaker.isOurError(err), false, 'isOurError() should return false' + ); }); breaker.fire(-1).then(result => { From 07abf6dbc86f3f8f47790b26df2466784bd0d186 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Thu, 17 Oct 2019 19:29:27 +0100 Subject: [PATCH 2/3] extract building error to function --- lib/circuit.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/circuit.js b/lib/circuit.js index 645cc830..ed16226c 100644 --- a/lib/circuit.js +++ b/lib/circuit.js @@ -374,8 +374,9 @@ class CircuitBreaker extends EventEmitter { * function. * * @return {Promise} promise resolves with the circuit function's return - * value on success or is rejected on failure of the action. Use isOurError() to - * determine if a rejection was a result of the circuit breaker or the action. + * value on success or is rejected on failure of the action. Use isOurError() + * to determine if a rejection was a result of the circuit breaker or the + * action. * * @fires CircuitBreaker#failure * @fires CircuitBreaker#fallback @@ -414,8 +415,7 @@ class CircuitBreaker extends EventEmitter { */ call (context, ...rest) { if (this.isShutdown) { - const err = tagError(new Error('The circuit has been shutdown.')); - err.code = 'ESHUTDOWN'; + const err = buildError('The circuit has been shutdown.', 'ESHUTDOWN'); return Promise.reject(err); } const args = Array.prototype.slice.call(rest); @@ -457,8 +457,7 @@ class CircuitBreaker extends EventEmitter { * @event CircuitBreaker#reject * @type {Error} */ - const error = tagError(new Error('Breaker is open')); - error.code = 'EOPENBREAKER'; + const error = buildError('Breaker is open', 'EOPENBREAKER'); this.emit('reject', error); @@ -476,10 +475,9 @@ class CircuitBreaker extends EventEmitter { timeout = setTimeout( () => { timeoutError = true; - const error = tagError( - new Error(`Timed out after ${this.options.timeout}ms`) + const error = buildError( + `Timed out after ${this.options.timeout}ms`, 'ETIMEDOUT' ); - error.code = 'ETIMEDOUT'; /** * Emitted when the circuit breaker action takes longer than * `options.timeout` @@ -531,8 +529,7 @@ class CircuitBreaker extends EventEmitter { } } else { const latency = Date.now() - latencyStartTime; - const err = tagError(new Error('Semaphore locked')); - err.code = 'ESEMLOCKED'; + const err = buildError('Semaphore locked', 'ESEMLOCKED'); /** * Emitted when the rate limit has been reached and there * are no more locks to be obtained. @@ -669,7 +666,9 @@ function fail (circuit, err, args, latency) { } } -function tagError (error) { +function buildError (msg, code) { + const error = new Error(msg); + error.code = code; error[OUR_ERROR] = true; return error; } From a8ea8fe85bd4de86d4c2301130d765802846c672 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Thu, 17 Oct 2019 19:30:00 +0100 Subject: [PATCH 3/3] linting --- test/test.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/test.js b/test/test.js index 6159db59..e250d3e9 100644 --- a/test/test.js +++ b/test/test.js @@ -209,7 +209,11 @@ test('Breaker opens after a configurable number of failures', t => { .then(t.fail) .catch(e => { t.equals(e.message, 'Breaker is open', 'breaker opens'); - t.equals(CircuitBreaker.isOurError(e), true, 'isOurError() should return true'); + t.equals( + CircuitBreaker.isOurError(e), + true, + 'isOurError() should return true' + ); }) .then(_ => breaker.shutdown()) .then(t.end); @@ -298,7 +302,9 @@ test('Passes error as last argument to the fallback function', t => { t.equals(result, `Error: ${fails} is < 0`, 'fallback received error as last parameter'); t.equals( - CircuitBreaker.isOurError(result), false, 'isOurError() should return false' + CircuitBreaker.isOurError(result), + false, + 'isOurError() should return false' ); breaker.shutdown(); t.end();