fix: apply apiRequestTimeout consistently across providers#567
fix: apply apiRequestTimeout consistently across providers#567daewoongoh wants to merge 4 commits into
Conversation
…c SDK providers Wire getApiRequestTimeout() into every provider that instantiates an OpenAI or Anthropic SDK client so the user-configured apiRequestTimeout setting applies uniformly. Previously only openai/lm-studio and providers extending BaseOpenAiCompatibleProvider honored it. OpenAI SDK: openai-native, openai-codex, openrouter, router-provider (lite-llm, zoo-gateway, opencode-go, vercel-ai-gateway), requesty, unbound, xai, qwen-code. Anthropic SDK: anthropic, minimax, anthropic-vertex.
Append the list of unsupported providers (Amazon Bedrock, Google Gemini, GCP Vertex AI, Mistral, Ollama, VS Code LM API, Poe) to the apiRequestTimeout setting description across all package.nls locales so users can see at a glance which providers ignore the value.
📝 WalkthroughWalkthroughThis PR imports and applies getApiRequestTimeout() across multiple provider client initializations, updates localization strings to list unsupported providers, and adjusts tests/mocks to expect the timeout option. ChangesProvider Timeout Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/package.nls.ca.json`:
- Line 41: The description for "settings.apiRequestTimeout.description" contains
a contradiction: it recommends higher timeouts for "Ollama" but also lists
"Ollama" among providers where the setting has no effect; edit that JSON string
to remove the contradiction by deleting "Ollama" from the unsupported providers
clause so it remains recommended as a local provider (ensure punctuation and
spacing remain valid in the JSON string).
In `@src/package.nls.id.json`:
- Line 41: The description for settings.apiRequestTimeout.description contains a
contradiction by naming "Ollama" both as a recommended local provider and as an
unsupported provider; choose one resolution and make it consistent across all
localization files by either removing "Ollama" from the recommendation clause
(e.g., "like LM Studio and Ollama") or removing it from the unsupported
providers list, then update the string for
settings.apiRequestTimeout.description in every localization file to reflect
that choice so the wording is no longer contradictory.
In `@src/package.nls.nl.json`:
- Line 41: The localization string for "settings.apiRequestTimeout.description"
contains contradictory guidance about Ollama; update the string so it's
consistent by removing "Ollama" from the recommendation clause (leave it in the
unsupported list), i.e., change the phrase "zoals LM Studio en Ollama" to
something like "zoals LM Studio" and ensure punctuation and spacing remain
correct around the comma so the key "settings.apiRequestTimeout.description"
reflects the resolved text.
In `@src/package.nls.tr.json`:
- Line 41: The description for "settings.apiRequestTimeout.description" wrongly
mentions Ollama as a local provider that may need higher timeout while also
listing it in the unsupported providers list; update the English source
(src/package.nls.json key settings.apiRequestTimeout.description) to remove
"Ollama" from the recommendation sentence (leave LM Studio and other genuine
local providers only) but keep "Ollama" in the unsupported-providers list, then
apply the same corrected text to all translated files including this file
(src/package.nls.tr.json) so the recommendation and unsupported lists are
consistent across locales.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 287b4f98-b9cb-46ce-aa25-f763d4bc6491
📒 Files selected for processing (29)
src/api/providers/anthropic-vertex.tssrc/api/providers/anthropic.tssrc/api/providers/minimax.tssrc/api/providers/openai-codex.tssrc/api/providers/openai-native.tssrc/api/providers/openrouter.tssrc/api/providers/qwen-code.tssrc/api/providers/requesty.tssrc/api/providers/router-provider.tssrc/api/providers/unbound.tssrc/api/providers/xai.tssrc/package.nls.ca.jsonsrc/package.nls.de.jsonsrc/package.nls.es.jsonsrc/package.nls.fr.jsonsrc/package.nls.hi.jsonsrc/package.nls.id.jsonsrc/package.nls.it.jsonsrc/package.nls.ja.jsonsrc/package.nls.jsonsrc/package.nls.ko.jsonsrc/package.nls.nl.jsonsrc/package.nls.pl.jsonsrc/package.nls.pt-BR.jsonsrc/package.nls.ru.jsonsrc/package.nls.tr.jsonsrc/package.nls.vi.jsonsrc/package.nls.zh-CN.jsonsrc/package.nls.zh-TW.json
Assert SDK clients receive a timeout option in providers updated by 4c82de4 (anthropic-vertex, openrouter, requesty, vercel-ai-gateway, zoo-gateway), and stub vscode.workspace.getConfiguration in tests that mock vscode as an empty object so getApiRequestTimeout() can run during provider construction.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The previous description recommended raising the timeout for "local providers like LM Studio and Ollama" while also listing Ollama among the providers where the setting has no effect. Drop the provider examples from the recommendation so the two parts no longer contradict; the unsupported list still spells out where the setting has no effect.
Related GitHub Issue
Closes: #565
Description
This PR makes the
zoo-code.apiRequestTimeoutsetting behave consistently across API providers.Previously, only a subset of providers explicitly propagated
getApiRequestTimeout()into their underlying SDK/client configuration, while others relied on default client behavior. As a result, changing the global timeout setting could produce different runtime behavior depending on the selected provider.Test Procedure
{ "zoo-code.apiRequestTimeout": 3600 }Pre-Submission Checklist
Screenshots / Videos
N/A
Documentation Updates
Additional Notes
This change is focused on consistency rather than introducing new timeout behavior. The goal is to ensure that the global
apiRequestTimeoutsetting is applied uniformly across provider implementations so users receive predictable behavior regardless of the selected provider.Providers without client-level timeout support
src/api/providers/bedrock.ts@aws-sdk/client-bedrock-runtimerequestTimeout: 0. Streaming semantics differ from OpenAI/Anthropic and are handled separately.src/api/providers/gemini.ts@google/genaitimeout; only per-requesthttpOptions.timeoutis supported. Would require threading the value into every call site.src/api/providers/vertex.ts@google/genai(extendsGeminiHandler)src/api/providers/mistral.ts@mistralai/mistralaitimeoutoption on the client.src/api/providers/native-ollama.tsollamanpm packagesrc/api/providers/vscode-lm.tssrc/api/providers/poe.tsai-sdk-provider-poe(Vercel AI SDK)createPoehas no client-leveltimeout; only per-callabortSignalis available.Get in Touch
hehegwk_23849
Summary by CodeRabbit
Bug Fixes
Documentation
Tests