fix: increase default API timeout from 5min to 10min#733
Closed
TimeToBuildBob wants to merge 4 commits intomasterfrom
Closed
fix: increase default API timeout from 5min to 10min#733TimeToBuildBob wants to merge 4 commits intomasterfrom
TimeToBuildBob wants to merge 4 commits intomasterfrom
Conversation
Implements Anthropic Claude native web search to address issue #492 (broken search due to bot detection on Google/DuckDuckGo). Features: - Environment variable configuration (GPTME_ANTHROPIC_WEB_SEARCH) - Configurable max_uses parameter (GPTME_ANTHROPIC_WEB_SEARCH_MAX_USES) - Seamless integration with existing tools - Automatic web search - Claude decides when to use it - Backward compatible (disabled by default) Implementation: - Added _create_web_search_tool() to create tool definition - Modified _prepare_messages_for_api() to conditionally include web search - Updated browser tool documentation - Added comprehensive test suite (3 new tests, all passing) Usage: export GPTME_ANTHROPIC_WEB_SEARCH=true gptme -m anthropic/claude-sonnet-4-5 Addresses: #492
The web search tool uses Anthropic's API structure (type, name, max_uses) which differs from the SDK's ToolParam TypedDict (name, description, input_schema). This is expected since web search is a newer feature where SDK types may lag API. Fixes typecheck errors at lines 276, 278, and 344.
…search - Add try/except for invalid GPTME_ANTHROPIC_WEB_SEARCH_MAX_USES values - Add duplicate tool check to prevent multiple web search tools - Add tests for both improvements - Addresses automated review feedback from PR #723 Changes: - Error handling: Falls back to default max_uses=5 if env var is invalid - Duplicate prevention: Checks tools_dict for existing web_search_20250305 - Test coverage: Added test_web_search_invalid_max_uses and test_web_search_no_duplicate_tools - All 5 web search tests passing
Addresses issue #732 where 5-minute default timeout was causing unnecessary stalls and retries. Changes: - Increased default timeout from 300s (5min) to 600s (10min) - Matches OpenAI and Anthropic client defaults - Updated comments to clarify this matches client defaults - Updated test expectations to match new 600s default Rationale: Both OpenAI and Anthropic Python clients have 10-minute defaults. By setting our own 5-minute default, we were being more restrictive than the clients themselves, causing requests that would succeed within 10 minutes to timeout at 5 minutes and then succeed on retry. The new 600s default aligns with client behavior while still allowing users to set custom timeouts via LLM_API_TIMEOUT environment variable. Fixes #732
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR addresses timeout issues by increasing the default API timeout from 5 minutes (300s) to 10 minutes (600s) for both OpenAI and Anthropic API clients. The change aligns gptme's timeout behavior with the native client library defaults, preventing unnecessary request failures and retries. The core issue was that gptme's custom 5-minute timeout was more restrictive than the underlying client defaults, causing requests to timeout prematurely and then succeed on retry when they just needed more time.
Additionally, the PR introduces Anthropic's native web search tool functionality, which provides a more reliable alternative to traditional web scraping methods that often get blocked by bot detection. This feature is configurable via environment variables and integrates cleanly with the existing tool system.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| gptme/llm/llm_openai.py | 5/5 | Updated default timeout from 300s to 600s to match OpenAI client defaults |
| gptme/llm/llm_anthropic.py | 4/5 | Updated timeout to 600s and added new native web search tool capability |
| tests/test_llm_openai.py | 5/5 | Updated test expectations to validate new 600-second timeout default |
| tests/test_llm_anthropic.py | 5/5 | Added comprehensive test coverage for new web search tool functionality |
| gptme/tools/browser.py | 5/5 | Added documentation for Anthropic's native web search configuration |
Confidence score: 4/5
- This PR is safe to merge with low risk of production issues
- Score reflects solid timeout fix with well-tested web search feature addition, but deducted one point due to the substantial new web search functionality that wasn't mentioned in the PR description and could benefit from more testing in production
- Pay close attention to gptme/llm/llm_anthropic.py for the new web search tool implementation and ensure the environment variable configuration works as expected
Sequence Diagram
sequenceDiagram
participant User
participant CLI/Client
participant LLM Module
participant Config
participant OpenAI Client
participant Anthropic Client
participant API
User->>CLI/Client: "Make API call with timeout configuration"
CLI/Client->>Config: "get_env('LLM_API_TIMEOUT')"
Config-->>CLI/Client: "timeout_str or None"
CLI/Client->>LLM Module: "init(provider, config)"
alt OpenAI Provider
LLM Module->>Config: "get_env('LLM_API_TIMEOUT')"
Config-->>LLM Module: "timeout_str or None"
LLM Module->>LLM Module: "timeout = float(timeout_str) if timeout_str else 600.0"
LLM Module->>OpenAI Client: "OpenAI(timeout=600.0)"
OpenAI Client-->>LLM Module: "client instance"
else Anthropic Provider
LLM Module->>Config: "get_env('LLM_API_TIMEOUT')"
Config-->>LLM Module: "timeout_str or None"
LLM Module->>LLM Module: "timeout = float(timeout_str) if timeout_str else 600.0"
LLM Module->>Anthropic Client: "Anthropic(timeout=600.0)"
Anthropic Client-->>LLM Module: "client instance"
end
LLM Module-->>CLI/Client: "client initialized"
CLI/Client->>LLM Module: "chat() or stream()"
LLM Module->>API: "API call with 10-minute timeout"
API-->>LLM Module: "response or timeout after 10 minutes"
LLM Module-->>CLI/Client: "response"
CLI/Client-->>User: "result"
Context used:
- Context from
dashboard- README.md file (source)
5 files reviewed, no comments
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 0d9a68a in 1 minute and 33 seconds. Click for details.
- Reviewed
342lines of code in5files - Skipped
0files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. gptme/llm/llm_anthropic.py:159
- Draft comment:
Default API timeout now defaults to 600s (from 300s), which aligns with client defaults. Consider whether you want to add explicit handling (or a clearer error message) when a non-numeric value is provided. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The first part about the timeout change is just informative and not actionable. The second part about error handling is a valid suggestion since the current code could crash with ValueError. However, this is a configuration value that would typically be set by developers/admins, not end users. The default value is sensible. The error would be obvious and easy to fix if it occurred. The suggestion for error handling could prevent crashes, but is it worth the added complexity for a developer-facing configuration value that rarely changes? While error handling would be more robust, the current behavior of crashing with ValueError is acceptable for a developer configuration. The stack trace would clearly show the issue. The comment should be deleted as it is partly informative (not actionable) and partly suggesting complexity that isn't clearly justified for this use case.
2. gptme/llm/llm_anthropic.py:488
- Draft comment:
The new _create_web_search_tool function is well implemented with proper parameters and return structure. It cleanly encapsulates the web search tool configuration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the implementation without offering any constructive criticism or questions.
3. gptme/llm/llm_anthropic.py:541
- Draft comment:
Integration of the native web search tool within _prepare_messages_for_api is clean. Duplicate prevention and error handling for invalid max_uses values are implemented nicely. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only praises the integration and implementation without providing any actionable feedback or suggestions for improvement. It does not align with the rules for useful comments.
4. gptme/llm/llm_openai.py:77
- Draft comment:
Timeout configuration is updated to 600s by default and the comments now match client defaults. This ensures consistency with external client libraries. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining a change that was made without suggesting any action or asking for clarification. It doesn't provide any actionable feedback or raise any concerns about the code.
5. tests/test_llm_anthropic.py:253
- Draft comment:
The tests for native web search functionality (enabled, disabled, invalid max_uses, duplicate prevention, and combination with other tools) are comprehensive and clearly validate the new behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it praises the comprehensiveness of the tests without providing any actionable feedback or suggestions for improvement. It does not align with the rules for useful comments.
6. tests/test_llm_openai.py:275
- Draft comment:
Timeout tests correctly verify that the default (600s) and custom timeouts are applied across all providers, and that invalid values raise errors as expected. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states that the timeout tests are correctly verifying certain conditions. It does not provide any actionable feedback or suggestions for improvement.
7. gptme/llm/llm_openai.py:80
- Draft comment:
Suggestion: Since both Anthropic and OpenAI modules perform similar timeout conversion logic, consider centralizing this logic into a shared utility to reduce duplication and ease future maintenance. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_ME9c5QVtpNttIaGD
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Member
Author
|
Closing in favor of PR #735 which implements Erik's preferred approach. Instead of explicitly setting timeout=600, PR #735 uses See Issue #732 for the full discussion. |
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
2 participants
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.
Summary
Fixes #732 by increasing the default API timeout from 5 minutes to 10 minutes.
Problem
The 5-minute default timeout was causing unnecessary stalls and retries:
Solution
Changed default timeout from 300s (5 min) to 600s (10 min) to match the OpenAI and Anthropic Python client defaults.
Changes
Research
Used Perplexity to verify client defaults:
Behavior
Before:
After:
LLM_API_TIMEOUTenv varTesting
test_timeout_defaultto expect 600sRelated
Important
Increase default API timeout from 5 to 10 minutes in
llm_anthropic.pyandllm_openai.py, aligning with client defaults and updating tests accordingly.llm_anthropic.pyandllm_openai.py.LLM_API_TIMEOUTenvironment variable.test_timeout_defaultintest_llm_openai.pyto expect 600s.test_llm_openai.py.llm_anthropic.pyandllm_openai.py.This description was created by
for 0d9a68a. You can customize this summary. It will automatically update as commits are pushed.