Skip to content

Conversation

@rawler
Copy link
Contributor

@rawler rawler commented Oct 13, 2025

This enables better categorization of errors.

In particular, TcpTransport translates ErrorKind to ureq::Error::Timeout errors on timeout conditions. These should then be re-raised as ErrorKind::Timedout errors behind the io::Error abstraction.

Omitting this error-structure forces downstream error-handlers to resort to string-matching to distinguish timeout-errors from other errors.

This enables better categorization of errors.

In particular, TcpTransport translates `ErrorKind` to
`ureq::Error::Timeout` errors on timeout conditions. These should then be
re-raised as `ErrorKind::Timedout` errors behind the `io::Error`
abstraction.

Omitting this error-structure forces downstream error-handlers to resort to
string-matching to distinguish timeout-errors from other errors.
@rawler
Copy link
Contributor Author

rawler commented Oct 13, 2025

Not sure this is necessarily the best way to solve it. But in particular, I've discovered some (but not all) errors disguising themselves as Fetch error: IO(Other, Some("timeout: receive response")) in production. In my application, these are kindof expected occasionally, and deserve slightly less aggressive logging than others.

@algesten
Copy link
Owner

@rawler thanks for submitting this.

I've been thinking back/forth on whether we can land it. In some respects it can be considered a breaking change.

@rawler
Copy link
Contributor Author

rawler commented Oct 16, 2025

Good reflection. I briefly touched on that thought too, but my thinking is that anyone relying on the old behavior would (hopefully) not be matching on ErrorKind=Other, but on string-matching the message, which I believe to be unchanged.

@rawler
Copy link
Contributor Author

rawler commented Oct 20, 2025

Are we reaching any verdict? Is there some other approach that would allow us to classify timeout errors (ideally without string-matching)?

@algesten
Copy link
Owner

I haven't forgotten, just not made up my mind.

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