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

example: HTTP circuit breaker doesn't consider HTTP status codes which are more indicative of retryable errors #46

Open
odeke-em opened this issue Mar 7, 2023 · 1 comment

Comments

@odeke-em
Copy link

odeke-em commented Mar 7, 2023

The circuit breaker pattern as per the linked docs is to allow retrying of operations that are less likely to fail. With the current code in example/ if I encounter a 404, it'll just return the 404 HTML page and moreover without any operational context hence defeating the pattern aimed for :-(

func Get(url string) ([]byte, error) {
body, err := cb.Execute(func() (interface{}, error) {
resp, err := http.Get(url)
if err != nil {
return nil, err
}
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return nil, err
}
return body, nil
})
if err != nil {
return nil, err
}
return body.([]byte), nil
}
func main() {
body, err := Get("http://www.google.com/robots.txt")
if err != nil {
log.Fatal(err)
}
fmt.Println(string(body))
}

There should be a way in which the example HTTP circuit breaker looks at the returned HTTP status codes. There are some for which you should not retry unless some conditions change e.g. authorization and authentication problems, not found and legal issue status codes.

At bare minimum please make the example more resilient with error checking such as

defer res.Body.Close()

if res.StatusCode/100 != 2 { // Not a 2XX successful response
    ... // Handle the status code in here
}
@rahulkhairwar
Copy link

Hi @odeke-em !
I believe your understanding of the circuit breaker pattern is not entirely correct.
This is what I got from ChatGPT (I was lazy to type 🥲)

To explain, the circuit breaker pattern is used to prevent cascading failures in distributed systems by detecting when a remote service or resource is unavailable, failing fast, and providing fallback behavior to prevent further requests to that service or resource. The pattern aims to increase the stability and resilience of the system.

Retry operations are often used in conjunction with the circuit breaker pattern, but they are not the main purpose of the pattern. Retry operations are typically used for transient errors that are expected to be resolved by retrying the operation.

Regarding your example, if the code encounters a 404 error, it should ideally handle it gracefully and provide meaningful feedback to the user. This can be achieved by providing a user-friendly error message along with the 404 HTML page, which can include operational context. This way, the user can understand what went wrong and take appropriate action.

However, this approach may not necessarily be related to the circuit breaker pattern. The circuit breaker pattern is primarily focused on preventing cascading failures in distributed systems, while error handling and feedback to users are important considerations in any software system.

Moreover, since the circuit-breaker pattern can wrap more than just HTTP calls, it is ideal to separate the logic handling via errors (passed to the IsSuccessful function, which can decide success/failure) rather than adding support for HTTP status codes to the core library.
In case of any further clarification, please do let me know! Happy to help :)

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

No branches or pull requests

2 participants