correctly expose HttpError through RetryError::source

[carlsverre]

Which issue does this PR close?

Closes #579

Rationale for this change

Consistently expose HttpError in the source chain for all http errors.

What changes are included in this PR?

The issue is caused by #[error(transparent)], which forwards the source method to the underlying error's source impl. The intention is that transparent results in the underlying error appearing to be embedded transparently within the embedding error. This means a user can't retrieve the HttpError via reflection.

I also added a test to verify that you can retrieve the HttpError in the source chain.

Are there any user-facing changes?

source will start returning HttpError before reqwest::Error when executed recursively. Hopefully, users recursively search for errors. This will break users who expect reqwest::Error to be at a fixed number of steps below the root error type.

[Copilot]

Pull request overview

This PR fixes an issue where HttpError was not directly accessible in the error source chain due to the use of #[error(transparent)] in the RequestError::Http variant. The change ensures that users can retrieve HttpError via reflection by using #[error("{0}")] instead, which makes HttpError appear directly in the source chain rather than being skipped in favor of its underlying error.

Key Changes:

• Modified RequestError::Http enum variant to use #[error("{0}")] instead of #[error(transparent)] to properly expose HttpError in the error source chain
• Added test to verify that HttpError can be found and accessed through the source chain
• Updated test imports to include HttpError and HttpErrorKind

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[alamb]

Nice find @carlsverre 🙏

[alamb]

I found this change confusing given the docs on https://docs.rs/thiserror/latest/thiserror/#derives

Errors may use error(transparent) to forward the source and Display methods straight through to an underlying error without adding an additional message. This would be appropriate for enums that need an “anything else” variant.

Specifically the documentation mostly discusses the display implications.

However, I verified that the newly added test fails without this change:

thread 'client::retry::tests::test_retry' (1398936) panicked at src/client/retry.rs:745:9:
HttpError not found in source chain

So in summary nice 🕵️ work @carlsverre (👋 btw -- it is great to see you on github)

[carlsverre]

Yea, perhaps the thiserror docs could be a bit better. I think it's somewhat of a footgun that source is also transparently forwarded, which effectively means the embedded #[from] Foo is no longer accessible via recursively searching source.

Nice to see you too!