fix(agents-api): API calls with 429, etc should be retried

[creatorrr]

Signed-off-by: Diwank Singh Tomer [email protected]

---

Important

Enhance API call retry logic for specific HTTP status codes and improve error handling in execute_api_call.

• Behavior:
  • execute_api_call in excecute_api_call.py now retries on HTTP status codes 408, 429, 503, and 504.
  • JSON parsing errors in execute_api_call are caught, logging a debug message and setting json to None.
• Exceptions:
  • Removed httpx.RequestError and httpx.HTTPStatusError from NON_RETRYABLE_ERROR_TYPES in tasks.py.
  • Updated is_non_retryable_error to retry on specific HTTP errors (408, 429, 503, 504).
• Testing:
  • Added workflow: tool call api_call test retry test in test_execution_workflow.py to verify retry logic for specific HTTP status codes.

^{This description was created by for 9b3fa19. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 9b3fa19 in 39 seconds

More details

• Looked at 167 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. agents-api/agents_api/activities/excecute_api_call.py:44

• Draft comment:
  Raising an exception for all HTTP status errors will prevent retries for status codes 408, 429, 503, and 504. Consider handling these status codes separately to allow retries.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment is relevant to the change made in the code, which is the addition of 'response.raise_for_status()'. This change will indeed prevent retries for certain status codes, which could be an issue if the intention is to allow retries for specific cases like 408, 429, 503, and 504. The comment is actionable as it suggests considering handling these status codes separately.
  The comment assumes that retries are necessary for these status codes, but without additional context, it's unclear if this is a requirement for this specific application. The comment could be speculative if the application does not need to handle retries for these status codes.
  Even if the application does not currently require retries for these status codes, the comment highlights a potential issue that could affect the robustness of the application. It's a valid consideration for future-proofing the code.
  The comment is relevant and actionable as it addresses a potential issue introduced by the code change. It should be kept to ensure the code handles HTTP status errors appropriately.

2. agents-api/agents_api/common/exceptions/tasks.py:22

• Draft comment:
  The FIXME comments suggest a need to refactor the retry logic to retry only on known errors. Consider addressing this in future updates.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The FIXME comments in agents_api/common/exceptions/tasks.py indicate a need for refactoring the retry logic. The current logic is to not retry on known errors, but the comment suggests it should be the opposite. This should be addressed in future updates.

3. agents-api/tests/test_execution_workflow.py:563

• Draft comment:
  The test correctly checks for retries but could be improved by verifying the exact number of retries expected for more precise validation.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The test workflow: tool call api_call test retry in agents_api/tests/test_execution_workflow.py is designed to check retry logic for specific status codes. It correctly verifies that retries occur, but it could be improved by checking the exact number of retries expected.

Workflow ID: wflow_5KLbvVoqQFFiUTa5

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

Catching BaseException is too broad and can lead to unintended behavior. Consider catching more specific exceptions.

[HamadaSalhab]

lgtm