-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Research and add defaults to HTTP resilience content #41611
Conversation
Co-authored-by: martintmk <103487740+martintmk@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. Let's
Additionally, these strategies handle the following exceptions: | ||
|
||
- `HttpRequestException` | ||
- `TimeoutRejectedException` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do the same thing for hedging? Just add circuit breaker exception too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow. So, the Hedging handler employs what defaults?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hedging has different ShouldHandle
predicate compared to standard resilience:
Maybe just note that status codes/exceptions are the same as compared to standard resilience, but hedging strategy also handles BrokenCircuitException
.
Summary
Researched and add defaults to HTTP resilience content
Fixes #41610 β CC: @martincostello
Internal previews