connor4312 / cockatiel

🐦 A resilience and fault-handling library. Supports Backoffs, Retries, Circuit Breakers, Timeouts, Bulkhead Isolation, and Fallbacks.
MIT License
1.59k stars 50 forks source link

[Question] Execution prevented because the circuit breaker is open #83

Closed JobaDiniz closed 11 months ago

JobaDiniz commented 11 months ago

I'm trying the following retry policy

timeout = timeout ?? 1000 * 30; //30s
        const maxAttempts = 5;
        //const policy = handleType(BrokenCircuitError).orWhenResult(element => element == undefined);
        const policy = handleAll.orWhenResult(element => element == undefined);
        const breakerPolicy = circuitBreaker(policy, { halfOpenAfter: timeout / maxAttempts, breaker: new ConsecutiveBreaker(maxAttempts / 2) });
        const retryPolicy = retry(policy, { maxAttempts: maxAttempts, backoff: new ExponentialBackoff({ maxDelay: timeout }) });

        const element = await wrap(retryPolicy, breakerPolicy).execute((context) => {
            //some code that returns an HTMLElement or undefined...
            return element;
        });

But it fails with Execution prevented because the circuit breaker is open.

What's wrong?

connor4312 commented 11 months ago

I don't see anything inherently wrong with that code.

JobaDiniz commented 11 months ago

I increased the initialDelay of the ExponentialBackoof to 500ms and it stopped giving me that error.

UPDATE: actually that did not solve. Depending on the internet speed, it throws that error. (I'm going to try to create a reproducible code)

connor4312 commented 11 months ago

I need more information about what you're trying to do and what you expect to see to offer any advice / bug fix. Can you share a minimal reproduction?

JobaDiniz commented 11 months ago

This is the minimal repo I could produce: https://github.com/JobaDiniz/cockatiel-retry

It's a chrome extension.

The readme says how to build it.

After build and loading into chrome, just click the "icon" extension button. There will be times when it will work. There will be times that it won't. Inspecting the Console you will see error.

When clicking the extension icon, it is supposed to:

  1. automatically click the "initiate request" button
  2. wait, using retries, for the last card to be rendered
  3. type in the input of the last rendered card.

Code: https://github.com/JobaDiniz/cockatiel-retry/blob/develop/src/content.ts#L27-L40

https://github.com/connor4312/cockatiel/assets/165290/161a6f5b-f62c-47d4-bc11-56e3e6e39e03

connor4312 commented 11 months ago

That seems pretty expected. With maxAttempts = 4, the circuit breaker is set to open after 2 failed attempts. That'll happen after about 500ms--the first check fails, then the next retry 500ms later will fail. Then after the next two attempts, bounced because the circuit breaker's half open time probably wasn't reached yet, the retry will give up.

JobaDiniz commented 11 months ago

So, how do I create a retry that won't fail on exception?

A retry with exponential backoof + a circuit breaker?

connor4312 commented 11 months ago

If you don't want it to ever stop retrying, set maxAttempts: Infinity

JobaDiniz commented 11 months ago

I want to stop retrying, but not with an error. I don't understand why it fails with exception. I come from dotnet, and maybe I'm using this typescript version wrong.

I want something like: retry, wait, retry, wait longer, retry again. That is, if the first couple of attempts fail, wait longer for the next ones.

By combining exponential backoff + circuit breaker, I thought I would achieve that.

connor4312 commented 11 months ago

I want to stop retrying, but not with an error.

Then you can use a try/catch. There's no mode in this library where retry would fail silently/return undefined if an inner function threw an error.

I want something like: retry, wait, retry, wait longer, retry again. That is, if the first couple of attempts fail, wait longer for the next ones.

That is what's happening, until the number of retries is exhausted. After 4 attempts, it allows the inner error/invalid result to go through. If you want to do this for longer, raise the number of maxAttempts.

JobaDiniz commented 11 months ago

Then you can use a try/catch. There's no mode in this library where retry would fail silently/return undefined if an inner function threw an error.

The inner function does not throw error. This would never throw error: return document.querySelector(selector) as HTMLElement;.

Are you saying I need to do this?

        try {
            const element = await wrap(retryPolicy, breakerPolicy).execute((context) => {
                return document.querySelector(selector) as HTMLElement;
            });
            return element;
        } catch (error) {
            return undefined;
        }

I thought the handleAll would catch all errors, no?

connor4312 commented 11 months ago

handleAll does catch all errors, but when retry exhausts all of its attempts, then the last error/errorful result is propagated. If the last error is one from the circuit breaker, then that's the error that gets thrown.