Repository navigation
Conversation
… error
LiteLLM raises DeepSeek's invalid-key 401 ("Authentication Fails, Your api
key: ... is invalid") as BadRequestError, but keeps the provider's status
code on the exception. looks_like_auth_error only matched message text, so
map_provider_exception (and profile validate) reported LLMBadRequestError.
Trust the 401/403 status code too, and drop the known-bug marker from the
F20.validate-bad-key-type map bullet.
Fixes #5862
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014MKndVj7XWvRQmvW9w2bDg
Coverage Report •
|
||||||||||||||||||||
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Summary
The fix is correct, minimal, and directly addresses the linked issue (#5862). In openhands-sdk/openhands/sdk/llm/exceptions/classifier.py, looks_like_auth_error now returns True for an openai.APIStatusError (which LiteLLM's BadRequestError is) carrying status_code 401/403, matching the existing typed AuthenticationError/PermissionDeniedError branch and the status 401/status 403 text check. The feature-map bullet loses its bug marker and the Gotchas line is removed.
Verification performed
- Root cause confirmed live. Against the real DeepSeek API on the pinned
litellm==1.93.0,completion(model='deepseek/deepseek-flash', api_key=<invalid>)raisesBadRequestErrorwithstatus_code == 401and the message "Authentication Fails, Your api key: … is invalid", which matches none ofAUTH_PATTERNSor thestatus 401/status 403text. On the base it mapped toLLMBadRequestError; on this headmap_provider_exceptionreturnsLLMAuthenticationError. The regression test does not depend on the network. - Acceptance criteria met. A plain 400
BadRequestErrorwithout auth wording still returnsLLMBadRequestError; a 401/403APIStatusErroris classified as auth whatever its message; a unit test covers it without a network call; theF20.validate-bad-key-typemarker is removed. - Tests run in the workspace.
tests/sdk/llm/test_exception_mapping.py,tests/sdk/llm/test_exception_classifier.py,test_auth_refresh_retry.py,test_budget_denial_http.py,test_llm_fallback.pyall pass; the broadertests/sdk/llmsuite is 1380 passed. The only failures aretests/sdk/llm/auth/test_credentials.py::test_get_credentials_dir_default/::test_get_credentials_dir_xdg, which fail becauseHOME=/var/openhands/.openhandsin this environment (the expected path doubles.openhands); they are unrelated to this diff. - CI on the exact head (
3cf4f8e9…). All check runs reportsuccess(pre-commit, smoke-test, integration-test, build-binary on ubuntu/macos/windows, coverage, OpenAPI breakage checks). No unresolved review threads. - The
openaiimport is consistent with existing usage elsewhere in the SDK (openhands/sdk/tool/tool.py,openhands/sdk/mcp/tool.py), so it introduces no new undeclared-dependency pattern.
Non-blocking residual risk (no action required)
map_provider_exception evaluates auth before content policy, so with the new 401/403 branch a ContentPolicyViolationError that carries a 403 response would now map to LLMAuthenticationError instead of LLMContentPolicyViolationError (I reproduced this with constructed state; e.g. ContentPolicyViolationError(..., response=<403>)). The normal inline content-policy case is a 400 and is unaffected. I found no evidence that a supported provider returns content-policy blocks with HTTP 403, so I am not treating this as a defect; flagging it only so the human maintainer is aware that the auth branch is now ordered ahead of content policy for 403 responses.
No material correctness, security, compatibility, or acceptance-criterion defect found on the current head.
✅ APPROVED
Co-authored-by: openhands <openhands@all-hands.dev>
|
📁 PR Artifacts Notice This PR contains a |
HUMAN:
This PR proposes a fix for a bug uncovered during the first full agentic run of
verify-agent-server.AGENT:
Summary
looks_like_auth_error(exception) existing typed authentication / permission checks + APIStatusError with status_code in {401, 403} → true existing message-pattern fallbackDeepSeek invalid key → LiteLLM BadRequestError(status=401) →
LLMAuthenticationError.Why
Some provider authentication failures arrive wrapped as BadRequestError and must still be classified as authentication errors.
Issue Number
Fixes #5862
How to Test
Previously recorded verification:
Reproduction commands, results, and limits
litellm.completion(model='deepseek/deepseek-flash', api_key='sk-qa-invalid', ...)raisesBadRequestErrorwithstatus_code401, andmap_provider_exceptionreturnedLLMBadRequestError.uv run pytest tests/sdk/llm/test_exception_mapping.py: the new test fails on main and passes here.uv run pytest tests/sdk/llm tests/agent_server/test_profiles_router.py tests/sdk/conversation/local/test_auth_error_friendly_message.py: 1382 passed.AGENT_SERVER_VERIFY_HOMEprivate, real DeepSeek):control-agent-server map run --file F20 --only F20.validate-bad-key-type --fresh --repeat 2 --checkout <main checkout>gavexfailtwice on main (6e7a853).xpasstwice.map checkreported no problems andmap run --only F20.validate-bad-key-type --freshgavepass.uv run pre-commit run --files <changed files>passes.Video/Screenshots
No UI change; reproduction evidence is above.
Type
Notes
The same classification also enables the existing managed-key refresh hook for these 401s. The regression test constructs the provider response without a network call; recorded live evidence is below. Related typed-status mapping: #3222.
Visual explanation by Codex using HumanLayer show-me.
🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdkpython-node-runtimepython-node-runtimepython-node-runtimegolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:3df6f1a-pythonRun
All tags pushed for this build
About Multi-Architecture Support
3df6f1a-python) is a multi-arch manifest supporting both amd64 and arm643df6f1a-python-amd64) are also available if needed