Skip to content

fix: consider timeout from hyper::Error#2742

Merged
seanmonstar merged 1 commit intoseanmonstar:masterfrom
flisky:master
Jun 23, 2025
Merged

fix: consider timeout from hyper::Error#2742
seanmonstar merged 1 commit intoseanmonstar:masterfrom
flisky:master

Conversation

@flisky
Copy link
Copy Markdown
Contributor

@flisky flisky commented Jun 20, 2025

I tried to push down std::io::ErrorKind::TimedOut into hyper::error::TimedOut's source, but failed (due to the referenced dyn Error). So I does here instead.

And another minor change: the test case didn't cover ErrorKind::TimedOut branch.

@seanmonstar
Copy link
Copy Markdown
Owner

Thanks for the PR! I'm guessing there was an instance you ran into because of this. Have you been able to test it out with a local patch override in your Cargo.toml?

@flisky
Copy link
Copy Markdown
Contributor Author

flisky commented Jun 23, 2025

Yes. I replace reqwest::Error::is_timeout with following ErrorExt::is_timeout2, and our customed retry logic works.

 pub trait ErrorExt {
     ...
+    fn is_timeout2(&self) -> bool;
 }

@@ -64,6 +69,13 @@ impl ErrorExt for reqwest::Error {
             })
     }

+    fn is_timeout2(&self) -> bool {
+        self.is_timeout()
+            || (self.is_request()
+                && get_source_error_type::<hyper::Error>(self)
+                    .is_some_and(hyper::Error::is_timeout))
+    }
+

Some background: the request error happens when the iOS phone turns on/off airplane mode, but reqwest::Error::is_timeout goes false:

error sending request for url (https://...): http2 error: keep-alive timed out

Copy link
Copy Markdown
Owner

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Thanks for the PR!

@seanmonstar seanmonstar merged commit 4cb2866 into seanmonstar:master Jun 23, 2025
37 checks passed
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

Successfully merging this pull request may close these issues.

2 participants