tustvold commented on code in PR #445:
URL:
https://github.com/apache/arrow-rs-object-store/pull/445#discussion_r2240594336
##########
src/client/http/connection.rs:
##########
@@ -101,27 +101,29 @@ impl HttpError {
// Reqwest error variants aren't great, attempt to refine them
let mut source = e.source();
- while let Some(e) = source {
- if let Some(e) = e.downcast_ref::<hyper::Error>() {
- if e.is_closed() || e.is_incomplete_message() ||
e.is_body_write_aborted() {
- kind = HttpErrorKind::Request;
- } else if e.is_timeout() {
- kind = HttpErrorKind::Timeout;
+ while kind == HttpErrorKind::Unknown {
Review Comment:
:+1: I think this should mean this only reclassifies errors that were
previously Unknown and doesn't potentially reclassify others.
##########
src/client/retry.rs:
##########
@@ -818,4 +825,55 @@ mod tests {
// Shutdown
mock.shutdown().await
}
+
+ #[tokio::test]
+ async fn test_connection_reset_is_retried() {
Review Comment:
I confirmed without the changes in this PR this test fails with
```
request should eventually succeed: RetryError(RetryErrorImpl { method: GET,
uri: Some(http://[::1]:43285/), retries: 0, max_retries: 2, elapsed: 662.178µs,
retry_timeout: 1s, inner: Http(HttpError { kind: Unknown, source:
reqwest::Error { kind: Request, source:
hyper_util::client::legacy::Error(Canceled, hyper::Error(Canceled,
hyper::Error(Io, Os { code: 104, kind: ConnectionReset, message: "Connection
reset by peer" }))) } }) })
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
