seanmonstar commented on code in PR #5383:
URL: https://github.com/apache/arrow-rs/pull/5383#discussion_r1489717884
##########
object_store/src/client/retry.rs:
##########
@@ -263,7 +262,7 @@ impl RetryExt for reqwest::RequestBuilder {
do_retry = true
} else if let Some(source) = e.source() {
if let Some(e) =
source.downcast_ref::<hyper::Error>() {
- if e.is_connect() || e.is_closed() ||
e.is_incomplete_message() {
+ if !(e.is_parse() || e.is_parse_status() ||
e.is_parse_too_large() || e.is_user() || e.is_canceled()) {
Review Comment:
> I can't help feeling if hyper doesn't expose this error variant, it
perhaps isn't one we should be handling.
> Perhaps @seanmonstar could clarify why those error kind types aren't
exposed?
I'd agree with that feeling. Anything accessed through `.source()` of a
hyper error is not guaranteed (we may change internal dependencies). Exposing
specific error classes is just something we haven't really designed (help
welcome).
(No opinion otherwise on this PR, other than the general be careful with
retrying unknown errors, especially if it could have meant side effects on the
server.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
