Skip to content

fix: prevent needless retry of TokenLimitExceeded and fix search engine fallback#1348

Open
Ricardo-M-L wants to merge 1 commit intoFoundationAgents:mainfrom
Ricardo-M-L:fix/retry-token-limit-and-search-fallback
Open

fix: prevent needless retry of TokenLimitExceeded and fix search engine fallback#1348
Ricardo-M-L wants to merge 1 commit intoFoundationAgents:mainfrom
Ricardo-M-L:fix/retry-token-limit-and-search-fallback

Conversation

@Ricardo-M-L
Copy link
Copy Markdown

Summary

This PR fixes two bugs:

Bug 1: TokenLimitExceeded retried 6 times despite intent not to retry it

File: app/llm.py (3 locations: ask, ask_with_images, ask_tool)

The @retry decorator uses retry_if_exception_type((OpenAIError, Exception, ValueError)). The comment says "Don't retry TokenLimitExceeded", but since TokenLimitExceeded inherits from Exception, tenacity matches it and retries 6 times — wasting tokens and time on a condition that cannot succeed on retry.

Fix: Remove Exception from the tuple. OpenAIError and ValueError already cover all intended retry cases. TokenLimitExceeded (which does not inherit from either) will now propagate immediately as intended.

Bug 2: Search engine fallback broken when an engine raises an exception

File: app/tool/web_search.py, method _try_all_engines

The call to _perform_search_with_engine() is not wrapped in try/except. If an engine raises an exception (e.g., network error, API key issue), the exception propagates up and the remaining fallback engines are never tried — defeating the purpose of the fallback mechanism.

Fix: Wrap the call in try/except Exception, log a warning with the engine name and error, append to failed_engines, and continue to the next engine. Also added failed_engines.append(engine_name) when search_items is empty so the final log message accurately reports all engines that were attempted.

Test plan

  • Verify TokenLimitExceeded is raised immediately without retries when the token limit is exceeded
  • Verify OpenAIError and ValueError are still retried up to 6 times
  • Verify that if the first search engine raises an exception, the next engine in the fallback order is tried
  • Verify that if all engines raise exceptions, the error is logged and an empty list is returned

🤖 Generated with Claude Code

…ne fallback

Bug 1: The retry decorator on ask(), ask_with_images(), and ask_tool() included
`Exception` in retry_if_exception_type alongside `OpenAIError` and `ValueError`.
Since `TokenLimitExceeded` inherits from `Exception`, it was retried 6 times
despite the comment saying otherwise. Removed `Exception` from the filter.

Bug 2: In _try_all_engines(), the call to _perform_search_with_engine() was not
wrapped in try/except. If an engine raised an exception, remaining fallback
engines were never tried. Added try/except to log the warning and continue to
the next engine.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Ricardo-M-L
Copy link
Copy Markdown
Author

Friendly bump — this fork PR's workflow run is stuck on action_required (GitHub's default for first-time contributors). Could a maintainer approve the workflow so CI can report a verdict? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant