mirror of
https://github.com/browser-use/browser-use.git
synced 2026-10-02 04:04:36 +08:00
fix(openrouter): validate client and response inputs (#5599)
## Summary
- keep `top_p` and `seed` on completion requests instead of passing them
to `AsyncOpenAI`
- load only `OPENROUTER_API_KEY` for OpenRouter and fail clearly when it
is missing
- turn empty `choices` responses into provider errors and preserve their
status codes
## Tests
- `uv run pytest -q tests/ci/models/test_llm_openrouter.py
tests/ci/test_openrouter_token_cost.py`
- `uv run pre-commit run --files browser_use/llm/openrouter/chat.py
tests/ci/models/test_llm_openrouter.py`
Closes #5598
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Fixes OpenRouter and Vercel AI Gateway client setup and response
handling so unsupported params stay on completion requests and
credential or response gaps surface as provider errors instead of
crashes. Closes #5598.
- `top_p` and `seed` go on completion requests, not to `AsyncOpenAI`.
- Only `OPENROUTER_API_KEY` is loaded; missing keys raise a 401 instead
of falling back to `OPENAI_API_KEY`.
- Empty `choices` raise a 502, and structured parse failures keep their
original status codes for both providers.
- Vercel AI Gateway now requires `AI_GATEWAY_API_KEY` or
`VERCEL_OIDC_TOKEN` and raises a 401 when missing.
<sup>Written for commit ca69ebdf89.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/browser-use/browser-use/pull/5599?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
This commit is contained in:
@@ -1,10 +1,11 @@
|
||||
import os
|
||||
from collections.abc import Mapping
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, TypeVar, overload
|
||||
|
||||
import httpx
|
||||
from openai import APIConnectionError, APIStatusError, AsyncOpenAI, RateLimitError
|
||||
from openai.types.chat.chat_completion import ChatCompletion
|
||||
from openai.types.chat.chat_completion import ChatCompletion, Choice
|
||||
from openai.types.shared_params.response_format_json_schema import (
|
||||
JSONSchema,
|
||||
ResponseFormatJSONSchema,
|
||||
@@ -57,17 +58,23 @@ class ChatOpenRouter(BaseChatModel):
|
||||
|
||||
def _get_client_params(self) -> dict[str, Any]:
|
||||
"""Prepare client parameters dictionary."""
|
||||
api_key = self.api_key or os.getenv('OPENROUTER_API_KEY')
|
||||
if not api_key:
|
||||
raise ModelProviderError(
|
||||
message='Missing OpenRouter API key. Set OPENROUTER_API_KEY or pass api_key.',
|
||||
status_code=401,
|
||||
model=self.name,
|
||||
)
|
||||
|
||||
# Define base client params
|
||||
base_params = {
|
||||
'api_key': self.api_key,
|
||||
'api_key': api_key,
|
||||
'base_url': self.base_url,
|
||||
'timeout': self.timeout,
|
||||
'max_retries': self.max_retries,
|
||||
'default_headers': self.default_headers,
|
||||
'default_query': self.default_query,
|
||||
'_strict_response_validation': self._strict_response_validation,
|
||||
'top_p': self.top_p,
|
||||
'seed': self.seed,
|
||||
}
|
||||
|
||||
# Create client_params dict with non-None values
|
||||
@@ -91,6 +98,15 @@ class ChatOpenRouter(BaseChatModel):
|
||||
self._client = AsyncOpenAI(**client_params)
|
||||
return self._client
|
||||
|
||||
def _get_first_choice(self, response: ChatCompletion) -> Choice:
|
||||
if response.choices:
|
||||
return response.choices[0]
|
||||
raise ModelProviderError(
|
||||
message='Invalid OpenRouter response: missing or empty `choices`.',
|
||||
status_code=502,
|
||||
model=self.name,
|
||||
)
|
||||
|
||||
@property
|
||||
def name(self) -> str:
|
||||
return str(self.model)
|
||||
@@ -154,9 +170,10 @@ class ChatOpenRouter(BaseChatModel):
|
||||
**(self.extra_body or {}),
|
||||
)
|
||||
|
||||
choice = self._get_first_choice(response)
|
||||
usage = self._get_usage(response)
|
||||
return ChatInvokeCompletion(
|
||||
completion=response.choices[0].message.content or '',
|
||||
completion=choice.message.content or '',
|
||||
usage=usage,
|
||||
)
|
||||
|
||||
@@ -185,7 +202,8 @@ class ChatOpenRouter(BaseChatModel):
|
||||
**(self.extra_body or {}),
|
||||
)
|
||||
|
||||
if response.choices[0].message.content is None:
|
||||
choice = self._get_first_choice(response)
|
||||
if choice.message.content is None:
|
||||
raise ModelProviderError(
|
||||
message='Failed to parse structured output from model response',
|
||||
status_code=500,
|
||||
@@ -193,13 +211,16 @@ class ChatOpenRouter(BaseChatModel):
|
||||
)
|
||||
usage = self._get_usage(response)
|
||||
|
||||
parsed = output_format.model_validate_json(response.choices[0].message.content)
|
||||
parsed = output_format.model_validate_json(choice.message.content)
|
||||
|
||||
return ChatInvokeCompletion(
|
||||
completion=parsed,
|
||||
usage=usage,
|
||||
)
|
||||
|
||||
except ModelProviderError:
|
||||
raise
|
||||
|
||||
except RateLimitError as e:
|
||||
raise ModelRateLimitError(message=e.message, model=self.name) from e
|
||||
|
||||
|
||||
@@ -348,6 +348,12 @@ class ChatVercel(BaseChatModel):
|
||||
def _get_client_params(self) -> dict[str, Any]:
|
||||
"""Prepare client parameters dictionary."""
|
||||
api_key = self.api_key or os.getenv('AI_GATEWAY_API_KEY') or os.getenv('VERCEL_OIDC_TOKEN')
|
||||
if not api_key:
|
||||
raise ModelProviderError(
|
||||
message='Missing Vercel AI Gateway API key. Set AI_GATEWAY_API_KEY or VERCEL_OIDC_TOKEN, or pass api_key.',
|
||||
status_code=401,
|
||||
model=self.name,
|
||||
)
|
||||
|
||||
base_params = {
|
||||
'api_key': api_key,
|
||||
@@ -661,6 +667,9 @@ class ChatVercel(BaseChatModel):
|
||||
stop_reason=response.choices[0].finish_reason if response.choices else None,
|
||||
)
|
||||
|
||||
except ModelProviderError:
|
||||
raise
|
||||
|
||||
except RateLimitError as e:
|
||||
raise ModelRateLimitError(message=e.message, model=self.name) from e
|
||||
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
"""Regression tests for OpenRouter client setup and response handling."""
|
||||
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from openai.types.chat import ChatCompletion, ChatCompletionMessage
|
||||
from openai.types.chat.chat_completion import Choice
|
||||
from pydantic import BaseModel
|
||||
|
||||
from browser_use.llm.exceptions import ModelProviderError
|
||||
from browser_use.llm.messages import UserMessage
|
||||
from browser_use.llm.openrouter.chat import ChatOpenRouter
|
||||
|
||||
|
||||
class Answer(BaseModel):
|
||||
answer: str
|
||||
|
||||
|
||||
def _completion(*, content: str | None = 'ok', choices: bool = True) -> ChatCompletion:
|
||||
return ChatCompletion(
|
||||
id='chatcmpl-test',
|
||||
choices=[Choice(finish_reason='stop', index=0, message=ChatCompletionMessage(role='assistant', content=content))]
|
||||
if choices
|
||||
else [],
|
||||
created=0,
|
||||
model='openai/gpt-4o',
|
||||
object='chat.completion',
|
||||
)
|
||||
|
||||
|
||||
async def test_request_params_reach_completion_not_client():
|
||||
llm = ChatOpenRouter(model='openai/gpt-4o', api_key='test-key', top_p=0.9, seed=42)
|
||||
|
||||
client = llm.get_client()
|
||||
|
||||
assert client.api_key == 'test-key'
|
||||
assert 'top_p' not in llm._get_client_params()
|
||||
assert 'seed' not in llm._get_client_params()
|
||||
|
||||
create = AsyncMock(return_value=_completion())
|
||||
with patch.object(type(client.chat.completions), 'create', create):
|
||||
await llm.ainvoke([UserMessage(content='question')])
|
||||
request_kwargs = create.await_args_list[0].kwargs
|
||||
assert request_kwargs['top_p'] == 0.9
|
||||
assert request_kwargs['seed'] == 42
|
||||
|
||||
|
||||
def test_provider_key_does_not_fall_back_to_openai_key(monkeypatch: pytest.MonkeyPatch):
|
||||
monkeypatch.setenv('OPENAI_API_KEY', 'wrong-provider-key')
|
||||
monkeypatch.setenv('OPENROUTER_API_KEY', 'openrouter-key')
|
||||
assert ChatOpenRouter(model='openai/gpt-4o').get_client().api_key == 'openrouter-key'
|
||||
|
||||
monkeypatch.delenv('OPENROUTER_API_KEY')
|
||||
with pytest.raises(ModelProviderError, match='Missing OpenRouter API key') as exc_info:
|
||||
ChatOpenRouter(model='openai/gpt-4o').get_client()
|
||||
assert exc_info.value.status_code == 401
|
||||
|
||||
|
||||
async def test_empty_choices_raise_provider_error():
|
||||
llm = ChatOpenRouter(model='openai/gpt-4o', api_key='test-key')
|
||||
create = AsyncMock(return_value=_completion(choices=False))
|
||||
|
||||
with patch.object(type(llm.get_client().chat.completions), 'create', create):
|
||||
with pytest.raises(ModelProviderError, match='missing or empty `choices`') as exc_info:
|
||||
await llm.ainvoke([UserMessage(content='question')])
|
||||
|
||||
assert exc_info.value.status_code == 502
|
||||
|
||||
|
||||
async def test_structured_provider_error_keeps_status_code():
|
||||
llm = ChatOpenRouter(model='openai/gpt-4o', api_key='test-key')
|
||||
create = AsyncMock(return_value=_completion(content=None))
|
||||
|
||||
with patch.object(type(llm.get_client().chat.completions), 'create', create):
|
||||
with pytest.raises(ModelProviderError, match='Failed to parse structured output') as exc_info:
|
||||
await llm.ainvoke([UserMessage(content='question')], Answer)
|
||||
|
||||
assert exc_info.value.status_code == 500
|
||||
@@ -0,0 +1,18 @@
|
||||
"""Regression tests for Vercel AI Gateway client setup."""
|
||||
|
||||
import pytest
|
||||
|
||||
from browser_use.llm.exceptions import ModelProviderError
|
||||
from browser_use.llm.vercel.chat import ChatVercel
|
||||
|
||||
|
||||
async def test_provider_key_does_not_fall_back_to_openai_key(monkeypatch: pytest.MonkeyPatch):
|
||||
monkeypatch.setenv('OPENAI_API_KEY', 'wrong-provider-key')
|
||||
monkeypatch.setenv('AI_GATEWAY_API_KEY', 'gateway-key')
|
||||
assert ChatVercel(model='openai/gpt-4o').get_client().api_key == 'gateway-key'
|
||||
|
||||
monkeypatch.delenv('AI_GATEWAY_API_KEY')
|
||||
monkeypatch.delenv('VERCEL_OIDC_TOKEN', raising=False)
|
||||
with pytest.raises(ModelProviderError, match='Missing Vercel AI Gateway API key') as exc_info:
|
||||
await ChatVercel(model='openai/gpt-4o').ainvoke([])
|
||||
assert exc_info.value.status_code == 401
|
||||
Reference in New Issue
Block a user