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

Add health check function to a circuit breaker #76

Merged
merged 3 commits into from
Jun 22, 2017
Merged

Conversation

lance
Copy link
Member

@lance lance commented Jun 21, 2017

Add circuit.healthCheck(func[, interval])

Exposes a way for users to provide their own health check function. The provided function
should return a promise. If the promise is rejected, the circuit will open and the health-check-failed event will be emitted.

Usage

const circuit = opossum(myFunction);
circuit.healthCheck(_ => {
    // check the status of the remote service
    if (remoteServiceDown) {
       return Promise.reject('Cannot connect to remote service');
    }
    return Promise.resolve();
  }, 10000); // check interval - default is 5000

circuit.on('health-check-failed', (err) => {
  // send a text message to the poor sysadmin
  sendErrorText(err);
});

@coveralls
Copy link

coveralls commented Jun 21, 2017

Coverage Status

Coverage increased (+0.02%) to 99.396% when pulling 0088ea8 on 52-health-check into 5b6bc96 on master.

Copy link
Member

@helio-frota helio-frota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good !
In fact I did not look in impl detail, but the usage seems very easy for the end user. 👍

@@ -151,6 +151,7 @@ Here are the events you can listen for.
* `halfOpen` - emitted when the breaker state changes to `halfOpen`
* `fallback` - emitted when the breaker has a fallback function and executes it
* `semaphore-locked` - emitted when the breaker is at capacity and cannot execute the request
* `health-check-failed` - emitted when a user-supplied health check function returns a rejected promise
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it also makes sense to have a Health check pass event.

I have no argument for or against :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but it seems like a lot of unnecessary noise. Since there are only two possible states, pass or fail, it seems like the absence of fail implies pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, probably no reason to have it. I'd say merge away 🎉

if (isNaN(interval)) throw new TypeError('Health check interval must be a number');

const check = _ => {
func.apply(this).catch(e => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related to my comment about the health-check-passed event, should we do something on non error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of have the same reasoning about this as I do about health-check-passed.

@lance lance merged commit 0e39faa into master Jun 22, 2017
@lance lance deleted the 52-health-check branch August 18, 2017 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants