sony / gobreaker

Circuit Breaker implemented in Go
MIT License
2.93k stars 185 forks source link

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

Open odeke-em opened 1 year ago

odeke-em commented 1 year ago

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 :-(

https://github.com/sony/gobreaker/blob/e7c4cd566971ee0c285b05c469e61e0993543edc/example/http_breaker.go#L26-L55

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 commented 1 year ago

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 :)