Skip to content

Report retry count on Ok results #235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

konstin
Copy link
Contributor

@konstin konstin commented Jun 7, 2025

Report retry count on Ok results that underwent retries through a RetryCount response extension, so that users can include the retry count in error messages.

We discovered in uv that we were not reporting retries for status code errors. This change does for Ok results what #159 did for Err results, enabling consistently showing retry messages in error traces (astral-sh/uv#13897).

Report retry count on `Ok` results that underwent retries through a `RetryCount` response extension, so that users can include the retry count in error messages.

We discovered in uv that we were not reporting retries for status code errors. This change does for `Ok` results what TrueLayer#159 did for `Err` results.
@konstin konstin requested a review from a team as a code owner June 7, 2025 10:54
konstin added a commit to astral-sh/uv that referenced this pull request Jun 16, 2025
Using a companion change in the middleware
(TrueLayer/reqwest-middleware#235, forked&tagged
pending review), we can check and show retries for HTTP status core
errors, to consistently report retries again.

We fix two cases:
* Show retries for status code errors for cache client requests
* Show retries for status code errors for Python download requests

Not handled:
* Show previous retries when a distribution download fails mid-streaming
* Perform retries when a distribution download fails mid-streaming
* Show previous retries when a Python download fails mid-streaming
* Perform retries when a Python download fails mid-streaming
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant