Per-request read timeout on KB::Client#request (1.1.0) - #105
Merged
Merged
Conversation
4 tasks
Add a `read_timeout:` keyword to `KB::Client#request` so a single call can raise its read budget above `KB.config.request.read_timeout`. Connect and write budgets stay global, and the override is applied through the Faraday request block, so it does not leak into later calls on the memoized connection. Motivation: `GET /v1/pets/birthdays` is the one KB endpoint whose server-side work legitimately runs for seconds (3.4s median, up to 39s in production), and it forced connected_health to run a 12s global timeout that no other call needs. With this seam the consumer can go back to the gem defaults and give only that call a longer budget. Bump to 1.1.0, document in README and CHANGELOG. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A local TCP server accepts and never answers; the call with read_timeout: 1 fails with Faraday::TimeoutError in about a second, well under the 5s default. WebMock is opened for localhost inside this spec only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fatbeard2
force-pushed
the
feat/per-request-read-timeout
branch
from
September 22, 2026 13:36
1235f50 to
3fc6779
Compare
fatbeard2
marked this pull request as ready for review
September 22, 2026 13:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
GET /v1/pets/birthdaysis the one Knowledge Base endpoint whose server-side work legitimately runs for seconds: 3.4s median, up to 39s in production last week. Because the gem only had a global read budget, connected_health had to run a 12s timeout for every KB call to keep that single daily job alive, while the steady-state p99 of all its other KB calls is 61ms. This gives the consumer a seam to go back to the gem defaults and raise the budget only where it is needed.Changes
KB::Client#requesttakesread_timeout:; when given, it is set on the Faraday request options for that call only. Connect and write budgets stay global.#requestaccept it too, for symmetry.Not in scope: retries.
max_retries = 0stays.Checklist
lib/kb/version.rbupdatedHow to test
spec/client_spec.rb#requestblock: default read timeout, the override, no leak into later calls, non-GET.spec/client_read_timeout_socket_spec.rbis the sanity check that the value reaches the socket: a local TCP server accepts the connection and never answers; the call withread_timeout: 1fails withFaraday::TimeoutErrorin about one second, well under the 5s default. WebMock is opened for localhost inside that spec only.Full suite 171 examples, 0 failures; RuboCop clean on lib/ and spec/ (ruby:3.2.3 container via docker-compose). CI (#106) runs both on this PR.
Consumer side: connected_health will point its Gemfile at this branch while testing, then at the released gem.
🤖 Generated with Claude Code