Skip to content

Conversation

@jr
Copy link
Collaborator

@jr jr commented Aug 6, 2025

Explicitly setting requestTimeout seems to be required for it to be possible to use the Bedrock provider with LiteLLM
Bedrock passthrough.

Example litellm config: https://gist.github.com/jr/bb8188c29719adb27565cffe45e0b5c3 Without this change trying to connect to LiteLLM using the bedrock provider with Bedrock API Key "sk-my_special_key" and custom VPC endpoint http://localhost:4000/bedrock will quickly fail. Seems like the requests are failing immediately for some reason.

I would love it if someone understood this more deeply, because it seems crazy.


Important

Adds requestTimeout: 0 to AwsBedrockHandler in bedrock.ts to fix LiteLLM passthrough issues.

  • Behavior:
    • Adds requestTimeout: 0 to clientConfig.requestHandler in AwsBedrockHandler in bedrock.ts to ensure compatibility with LiteLLM passthrough.
  • Context:
    • Without this change, requests to LiteLLM using the Bedrock provider fail immediately.

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

Explicitly setting requestTimeout seems to be required for it to be
possible to use the Bedrock provider with LiteLLM
Bedrock passthrough.
@jr jr requested review from cte and mrubens as code owners August 6, 2025 23:02
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Aug 6, 2025
Copy link
Contributor

@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 have some suggestions for improvement. The workaround appears functional, but understanding the root cause better and considering potential side effects would strengthen this fix.

clientConfig.requestHandler = {
// This should be the default anyway, but without setting something
// this provider fails to work with LiteLLM passthrough.
requestTimeout: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment mentions "This should be the default anyway" but it's unclear why explicitly setting requestTimeout: 0 fixes the LiteLLM issue. Could you provide more context about what was happening without this setting? Was LiteLLM timing out immediately, or was there a different default being applied?

Also, setting requestTimeout: 0 means no timeout limit. While this fixes the immediate issue, could this potentially cause requests to hang indefinitely in certain failure scenarios? Would a reasonable timeout value (e.g., 300000 for 5 minutes) work instead?

// This should be the default anyway, but without setting something
// this provider fails to work with LiteLLM passthrough.
requestTimeout: 0,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a workaround for a specific LiteLLM passthrough issue, consider adding a test case that verifies the requestHandler configuration is properly set when using API key authentication. This would prevent regression if the code is refactored in the future.

Also, would it make sense to add a configuration option like awsLiteLLMCompatibilityMode so users can explicitly enable this workaround only when needed?

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 6, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 9, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Aug 9, 2025
@hannesrudolph hannesrudolph added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Aug 9, 2025
@mrubens mrubens merged commit cdc31f7 into main Aug 9, 2025
28 checks passed
@mrubens mrubens deleted the jr/bedrock-litellm-passthrough branch August 9, 2025 18:29
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Aug 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 9, 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 lgtm This PR has been approved by a maintainer PR - Needs Review size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants