Skip to content

fix(bedrock): reject per-request timeout with a custom client - #1297

Merged
tbille merged 1 commit into
mozilla-ai:mainfrom
tonycoder-hub:cursor/bedrock-custom-client-timeout-f3ff
Aug 18, 2026
Merged

tbille merged 1 commit into
mozilla-ai:mainfrom
tonycoder-hub:cursor/bedrock-custom-client-timeout-f3ff

Conversation

@tonycoder-hub

@tonycoder-hub tonycoder-hub commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #1264.

Bedrock with a caller-supplied client= ignored per-request timeout (TIMEOUT_SUPPORT is mapped). Raise UnsupportedParameterError when timeout is set alongside a custom client, matching the centralized rejection. Provider-built clients are unchanged.

PR Type

  • Bug Fix

Relevant issues

Fixes #1264

Checklist

  • I understand the code I am submitting.
  • I have added unit tests that prove my fix/feature works
  • I have run this code locally and verified it fixes the issue.
  • New and existing tests pass locally
  • Documentation was updated where necessary
  • I have read and followed the contribution guidelines
  • AI Usage:
    • This is fully AI-generated.

AI Usage Information

  • AI Model used: Claude Opus 5
  • AI Developer Tool used: Cursor cloud agent
  • I am an AI Agent filling out this form (check box if true)

tests/unit/providers/test_aws_provider.py timeout/custom_client: 12 passed. tests/unit: 2158 passed.

Summary by CodeRabbit

  • Bug Fixes
    • Custom Bedrock clients now clearly reject unsupported per-request timeout settings instead of silently ignoring them.
    • Provider-managed Bedrock clients continue to support configured timeouts.
    • Custom clients remain available for synchronous, streaming and asynchronous requests when no timeout is specified.

A per-request `timeout` is honored by routing the call through a boto3 client built
with matching connect/read timeouts. That is impossible for a caller-supplied
`client=`, whose construction any-llm does not own, so the value was dropped with a
warning. Bedrock declares `TIMEOUT_SUPPORT = "mapped"`, which promises the timeout is
translated rather than discarded, so the call could run unbounded while the caller
believed a deadline was in force.

Raise `UnsupportedParameterError` on that path instead, matching how the centralized
timeout handling rejects an unsupported parameter, and point callers at
`botocore.config.Config`.

Fixes mozilla-ai#1264

Co-authored-by: Tony Coder <407243179@qq.com>
@coderabbitai

coderabbitai Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1eb4f671-a932-4be6-ba8c-59a7393fa5ea

📥 Commits

Reviewing files that changed from the base of the PR and between c0f3ffb and 553485a.

📒 Files selected for processing (2)
  • src/any_llm/providers/bedrock/bedrock.py
  • tests/unit/providers/test_aws_provider.py

Included review availability: Your plan includes up to 8 reviews per rolling hour; 6 remain after this review.


Walkthrough

Bedrock custom clients now raise UnsupportedParameterError when a per-request timeout is supplied. Synchronous, streaming, and asynchronous tests cover rejection. Requests without a timeout continue to use the custom client.

Changes

Bedrock custom-client timeout handling

Layer / File(s) Summary
Custom-client timeout rejection
src/any_llm/providers/bedrock/bedrock.py
Custom Bedrock clients raise UnsupportedParameterError for per-request timeouts. The timeout-support documentation describes this behaviour.
Timeout regression coverage
tests/unit/providers/test_aws_provider.py
Tests cover synchronous, streaming, and asynchronous rejection. Tests also confirm that requests without a timeout use the custom client.

Suggested reviewers: tbille

Merge Risk: ⚪ Minimal · up to 55348

The change rejects unsupported per-request timeouts when using a custom Bedrock client while preserving provider-built client behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting per-request timeouts when using a custom Bedrock client.
Description check ✅ Passed The description follows the required template, identifies issue #1264, explains the fix, records testing, and completes the checklist and AI usage details.
Linked Issues check ✅ Passed The implementation satisfies issue #1264 by raising UnsupportedParameterError for custom-client timeouts while preserving provider-built client behaviour and adding regression tests.
Out of Scope Changes check ✅ Passed The changes are limited to Bedrock timeout handling and related unit tests, with no unrelated changes identified.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/any_llm/providers/bedrock/bedrock.py 90.06% <100.00%> (-5.42%) ⬇️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tbille
tbille merged commit 244804c into mozilla-ai:main Aug 18, 2026
16 checks passed
tbille pushed a commit that referenced this pull request Aug 18, 2026
## Description
Bedrock and SageMaker `_convert_params` gated `temperature` and `top_p`
on truthiness, so `0.0` was dropped and the request used the model
default (e.g. Claude 1.0 on Bedrock) instead of greedy decoding. Other
providers already use `is not None`.

`max_tokens` and `stop` stay on truthiness: `0` and empty sequences are
rejected by both providers.

## PR Type
- Bug Fix

## Relevant issues
No open issue. Distinct from #1292 (Anthropic tool_choice) and our
#1296/#1297/#1298.

## Checklist
- [x] I understand the code I am submitting.
- [x] I have added unit tests that prove my fix/feature works
- [x] I have run this code locally and verified it fixes the issue.
- [x] New and existing tests pass locally
- [x] Documentation was updated where necessary
- [x] I have read and followed the contribution guidelines
- [x] **AI Usage:**
    - [x] This is fully AI-generated.

## AI Usage Information
- AI Model used: Claude Opus 5
- AI Developer Tool used: Cursor cloud agent
- [x] I am an AI Agent filling out this form (check box if true)

`uv run pytest tests/unit` → 2161 passed, 69 skipped. Target files fail
on current main (`KeyError: inferenceConfig` / missing keys) and pass
after.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* AWS Bedrock and SageMaker integrations now correctly honour explicitly
configured zero values for temperature and top-p sampling parameters.
  * Unset sampling parameters continue to be omitted from requests.

* **Tests**
* Added coverage confirming zero-valued parameters are forwarded and
unset values are excluded.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
JamMaster1999 added a commit to JamMaster1999/any-llm that referenced this pull request Aug 18, 2026
Brings in upstream's merges of our mozilla-ai#1291/mozilla-ai#1292/mozilla-ai#1310 plus mozilla-ai#1297, mozilla-ai#1299,
mozilla-ai#1301, mozilla-ai#1302, mozilla-ai#1303, mozilla-ai#1305. Carried-until-merged fork work stays:
mozilla-ai#1294 (gemini reasoning_effort=none), mozilla-ai#1308 (aresponses timeout),
mozilla-ai#1309 (gemini native tool dicts), and the mozilla-ai#1300 carry.
One conflict in tests/unit/test_responses.py: kept our mozilla-ai#1308 timeout
test next to upstream's flatten test. Unit suite: 2234 passed.

Claude-Session: https://claude.ai/code/session_018D3FGNvb1hRZQmsXFoA44J
@github-actions github-actions Bot added the 1.27.0 Included in release 1.27.0 label Sep 3, 2026

This branch was previously deployed

1 inactive deployment
integration-tests — 553485ad Deployed Aug 18, 2026 by tonycoder-hub via run-docs-tests #2452
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.27.0 Included in release 1.27.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bedrock: align custom-client per-request timeout with centralized TIMEOUT_SUPPORT rejection

3 participants