Preserve nil timeouts through request transport#293
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28a082a4b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| pool.with(timeout: timeout, &blk) | ||
| if deadline.nil? | ||
| pool.with(&blk) |
There was a problem hiding this comment.
Preserve nil timeout while waiting for pooled connections
When timeout: nil is used and all connections for an origin are checked out, calling pool.with without an explicit timeout falls back to connection_pool's default checkout timeout (5 seconds). That means a request that disabled timeouts can still raise a timeout while queued for a pooled connection, even though README says queue time counts toward the request timeout and nil disables it.
Useful? React with 👍 / 👎.
Summary
timeout: nilas "no timeout" instead of coercing it to0.0x-stainless-timeouthandling when a request disables timeoutsNet::HTTPsocket timeouts and add regressions for default and per-request nil timeout behaviorWhy
The README documents
timeout: nilas disabling timeouts, but the transport currently callsto_fon the timeout value. In Ruby,nil.to_fbecomes0.0, which turns the computed deadline into "right now" rather than removing the deadline.Validation
ASDF_RUBY_VERSION=3.4.9 asdf exec bundle exec ruby -Itest test/openai/client_test.rb --name=/timeout/ASDF_RUBY_VERSION=3.4.9 asdf exec bundle exec ruby -Itest test/openai/client_test.rb