Skip to content

Comments

Retry timeout errors for streams #17735

Merged
zanieb merged 1 commit intokonsti/connect-timeout-and-retryfrom
konsti/retry-timeouts
Feb 4, 2026
Merged

Retry timeout errors for streams #17735
zanieb merged 1 commit intokonsti/connect-timeout-and-retryfrom
konsti/retry-timeouts

Conversation

@konstin
Copy link
Member

@konstin konstin commented Jan 29, 2026

We're already retrying most timeout errors (only retry_read_timeout_stream didn't retry before), but not for streams after we already received some bytes which this PR adds.

@konstin konstin requested a review from EliteTK January 29, 2026 15:29
@konstin konstin added enhancement New feature or improvement to existing functionality network Network connectivity e.g. proxies, DNS, and SSL labels Jan 29, 2026
@konstin konstin force-pushed the konsti/connect-timeout-and-retry branch from ecb43c2 to 2ba2382 Compare February 2, 2026 12:17
@konstin konstin force-pushed the konsti/retry-timeouts branch from cfe8280 to 64587fc Compare February 2, 2026 12:19
Copy link
Contributor

@EliteTK EliteTK left a comment

Choose a reason for hiding this comment

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

It looks like only retry_read_timeout_stream fails if I remove the fix.

retry_read_timeout_index and connect_timeout_stream pass without the changes.

(Confirmed with @konstin via side channel that this doesn't seem intended so I'll postpone a full review until he checks this.)

├─▶ Request failed after 1 retry
├─▶ Failed to read metadata: `http://[LOCALHOST]/tqdm-0.1-py3-none-any.whl`
├─▶ Failed to read from zip file
├─▶ an upstream reader returned an error: Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: [TIME]).
Copy link
Contributor

Choose a reason for hiding this comment

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

Not something introduced by this PR but I noticed there is inconsistent casing on these errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the an upstream reader returned an error is not from us, hence the different casing.

@konstin konstin force-pushed the konsti/connect-timeout-and-retry branch from 2ba2382 to 1f1321d Compare February 3, 2026 20:07
@konstin konstin force-pushed the konsti/retry-timeouts branch from 64587fc to 450b082 Compare February 3, 2026 21:28
@konstin konstin force-pushed the konsti/connect-timeout-and-retry branch 2 times, most recently from c83593b to 31e94f6 Compare February 4, 2026 16:20
@konstin konstin force-pushed the konsti/retry-timeouts branch from 450b082 to be26e10 Compare February 4, 2026 16:22
@zanieb zanieb merged commit d19338a into konsti/connect-timeout-and-retry Feb 4, 2026
118 checks passed
@zanieb zanieb deleted the konsti/retry-timeouts branch February 4, 2026 19:01
@zanieb
Copy link
Member

zanieb commented Feb 4, 2026

Ah sorry I missed that this was stacked 😭

konstin added a commit that referenced this pull request Feb 5, 2026
Rebase of #17735

We're already retrying most timeout errors (only
`retry_read_timeout_stream` didn't retry before), but not for streams
after we already received some bytes which this PR adds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or improvement to existing functionality network Network connectivity e.g. proxies, DNS, and SSL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants