Repository navigation
fix(aws,bedrock): raise CredentialsError when the session resolves no credentials - #2059
Steve-20-ai wants to merge 1 commit into
Conversation
…dentials The signing helper moved to lib/aws/_auth.py and is now shared by the Bedrock and AWS (mantle) clients, so the guard lives there and both clients get the error class.
c4bfe5f to
053e5c2
Compare
|
Rebuilt on credentials = session.get_credentials()
if not credentials:
raise CredentialsError("Could not resolve AWS credentials from session")The diff is back to Two things changed beyond the port:
Red/green on the rebuilt tree, The four failures in both runs are the |
mangeshraut712
left a comment
There was a problem hiding this comment.
Mapping a missing session to CredentialsError is the right class: it is an AnthropicError and not an APIConnectionError, so except AnthropicError works and the client will not retry a config problem. The tests only cover get_credentials() returning None; the live boto3/botocore path can fail in other ways.
| credentials = session.get_credentials() | ||
| if not credentials: | ||
| raise RuntimeError("Could not resolve AWS credentials from session") | ||
| raise CredentialsError("Could not resolve AWS credentials from session") |
There was a problem hiding this comment.
session.get_credentials() is not guaranteed to return None. botocore often raises botocore.exceptions.NoCredentialsError (or TokenRetrievalError / UnauthorizedSSOTokenError for SSO and refresh failures) instead of a silent miss. Those would still escape as raw botocore exceptions, so the except CredentialsError / except AnthropicError contract this PR is aiming for would not hold on a real empty or broken credential chain — only on the mocked return None path.
I'd catch those botocore errors here and re-raise CredentialsError (with the original as __cause__), and add a test that stubs get_credentials to raise NoCredentialsError. Worth a quick grep for any remaining RuntimeError("Could not resolve AWS credentials") in tests/lib/test_aws_auth.py or other signing helpers so the swap is complete.
agammann
left a comment
There was a problem hiding this comment.
get_auth_headers is shared by Bedrock and AnthropicAWSClient, so this also changes the latter’s public exception type. The new tests exercise only AnthropicBedrock and AsyncAnthropicBedrock; could a client call through AnthropicAWSClient with get_credentials() returning None assert it raises CredentialsError? That would cover the second affected client path.
Problem
AnthropicBedrock/AsyncAnthropicBedrockwith no resolvable AWS credentials raise a builtinRuntimeErrorfrom the signing path:anthropic>=2already shipsanthropic.CredentialsError(AnthropicError)— documented as "credentials cannot be loaded from local files, config, or the environment", which is exactly this case — but the signing helper raises a bareRuntimeErrorinstead. Callers and wrappers therefore cannot classify a missing-credentials configuration problem withexcept anthropic.AnthropicError, and it is not turned intoAPIConnectionErroreither, because signing runs in_prepare_request()before thetryaroundself._client.send(...).Fix
CredentialsErroris what the SDK defines for this situation, it is anAnthropicError, and it is deliberately not aRetryableError— retrying cannot conjure credentials, so the error surfaces immediately instead of burningmax_retries.Compatibility
RuntimeErroris not currently documented for this path, andCredentialsErrorsubclassesAnthropicError, notRuntimeError, so code that catches the old bareRuntimeErrorspecifically will stop matching. That is the point of the change, but it is worth a release note. Everything that catchesAnthropicError— the documented way to catch SDK errors — now catches this case for the first time.Rebuilt on the current
mainThe first revision patched
src/anthropic/lib/bedrock/_auth.py. That module no longer exists: the SigV4 signing helper was lifted tosrc/anthropic/lib/aws/_auth.pyand is now shared by the Bedrock client and the AWS/mantle client (lib/bedrock/_client.pyimportsfrom ..aws._auth import get_auth_headers,lib/aws/_client.pydoes the same). The guard is the same line, in the new home:So the branch is rebuilt on
mainatd4976414and the one-line change now sits inlib/aws/_auth.py. A side effect worth noting:lib/aws/_client.pyreaches the same helper, so anAnthropicAWSClientwith no resolvable credentials now also getsCredentialsErrorrather thanRuntimeError. That is the same class of problem, and the alternative — leaving that client onRuntimeError— would mean two error types for one condition.The tests patch
anthropic.lib.aws._auth._get_sessionrather than the old bedrock module, and there is now a second test for the async client, since the async path goes throughasyncify(get_auth_headers)and could plausibly wrap the error differently.Validation
Both tests use a session whose
get_credentials()returnsNone, so they do not depend on the ambient AWS configuration of the machine running them.The four remaining failures are identical on both sides:
test_region_infer_from_profileand thetest_region_infer_from_specified_profile/test_region_infer_from_aws_profile_argumentparameters. They read~/.aws/configand fail withProfileNotFound/ValueErrorin this Windows sandbox where no such profile exists; they are unrelated to the change and untouched by it.ruff checkandruff format --checkon both files: clean.Adjacent issues not covered here
lib/bedrock/_client.pyalso raises a bareRuntimeError("Expected dictionary json_data for post /v1/messages endpoint"). That one is an internal invariant violation rather than a configuration problem, soCredentialsErrorwould be the wrong class; left alone.ValueError("No AWS region was provided")are untouched.AnthropicAWSClientis covered by the same one-line change but has no test of its own here; it would need the same_get_sessionstub againstlib/aws/_client.py.Agent involvement
Written with an agent (WorkBuddy coding agent, DeepSeek-V4.1-Flash). The reproduction, the red/green numbers above and the diff were produced and verified locally, the rebuild against the current
mainincluded.