Skip to content

Conversation

elianiva
Copy link

@elianiva elianiva commented Sep 5, 2025

Related GitHub Issue

Closes: #7366

Roo Code Task Context (Optional)

Description

This PR fixes timeout to match the docs. This PR makes it so that 0 actually disables the timeout by setting it to max 32 int value (2^31 - 1).

I wonder if I should change the getApiRequestTimeout instead to return that value? I keep all of it at the providers right now. Tell me if that is more preferable than the current implementation.

Test Procedure

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch

@elianiva


Important

Standardize timeout behavior across providers by setting it to 2147483647 when getApiRequestTimeout() returns 0, effectively disabling the timeout.

  • Behavior:
    • Sets timeout to 2147483647 (2^31 - 1) when getApiRequestTimeout() returns 0, effectively disabling the timeout.
    • Applied to AnthropicHandler, BaseOpenAiCompatibleProvider, and AwsBedrockHandler constructors, among others.
  • Functions:
    • getApiRequestTimeout() used to fetch timeout value in anthropic.ts, base-openai-compatible-provider.ts, and bedrock.ts.
  • Misc:
    • Comments added to explain the rationale for using 2147483647 as the maximum timeout value.

This description was created by Ellipsis for 147ec36. You can customize this summary. It will automatically update as commits are pushed.

@elianiva elianiva requested review from mrubens, cte and jr as code owners September 5, 2025 14:52
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug Something isn't working labels Sep 5, 2025
Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! I've reviewed the changes and found some issues that need attention before this can be merged.

Copy link

roomote bot commented Sep 5, 2025

Important: The test file src/api/providers/__tests__/openai-timeout.spec.ts needs to be updated. Line 140 currently expects timeout to be 0 when getApiRequestTimeout returns 0, but with your changes it should now expect 2147483647. This will cause test failures if not updated.

You may also want to add tests to verify that providers correctly transform 0 to 2147483647 for better test coverage of this new behavior.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 5, 2025
@elianiva
Copy link
Author

elianiva commented Sep 5, 2025

on second thought, there's no reason not to do that, brb will update it

@daniel-lxs daniel-lxs moved this from Triage to PR [Draft / In Progress] in Roo Code Roadmap Sep 6, 2025
@hannesrudolph hannesrudolph added PR - Draft / In Progress and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR - Draft / In Progress size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
Status: PR [Draft / In Progress]
Development

Successfully merging this pull request may close these issues.

Roo timeouts with LM Studio
2 participants