Skip to content

Fix OpenRouter header provider argument handling #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix OpenRouter header provider argument handling #647

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 63 additions & 16 deletions src/connectors/openrouter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

import logging
from collections.abc import Callable
from collections.abc import Callable, Mapping
from typing import Any, cast

import httpx
Expand Down Expand Up @@ -58,6 +58,22 @@ def _build_openrouter_header_context(self) -> dict[str, str]:

return {"app_site_url": referer, "app_x_title": title}

@staticmethod
def _authorization_includes_api_key(
headers: Mapping[str, str], api_key: str | None
) -> bool:
"""Check whether the Authorization header contains the expected API key."""

if not api_key:
return True

for header_name, value in headers.items():
if header_name.lower() == "authorization" and isinstance(value, str):
if api_key in value:
return True

return False

def _resolve_headers_from_provider(self) -> dict[str, str]:
"""Call the configured headers provider with appropriate arguments."""
if not self.headers_provider or not self.api_key:
Expand All @@ -69,26 +85,57 @@ def _resolve_headers_from_provider(self) -> dict[str, str]:
provider = self.headers_provider
errors: list[Exception] = []

if self.key_name is not None:
def _try_provider_call(*args: Any) -> dict[str, str] | None:
try:
return provider(self.key_name, self.api_key)
result = provider(*args)
except (AttributeError, TypeError) as exc:
errors.append(exc)
return None
except Exception as exc:
errors.append(exc)
return None

context = self._build_openrouter_header_context()
try:
return provider(context, self.api_key)
except Exception as exc: # pragma: no cover - should not happen in normal flow
if errors:
logger.debug(
"Headers provider rejected key_name input: %s",
errors[-1],
exc_info=True,
if not isinstance(result, Mapping):
errors.append(
TypeError("OpenRouter headers provider must return a mapping."),
)
raise AuthenticationError(
message="OpenRouter headers provider failed to produce headers.",
code="missing_credentials",
) from exc
return None

headers = dict(result)
if not self._authorization_includes_api_key(headers, self.api_key):
errors.append(
ValueError(
"OpenRouter headers provider did not include API key in Authorization header.",
)
)
return None

return headers

if self.key_name is not None:
headers = _try_provider_call(self.key_name, self.api_key)
if headers is not None:
return headers

headers = _try_provider_call(self.api_key, self.key_name)
if headers is not None:
return headers

context = self._build_openrouter_header_context()
headers = _try_provider_call(context, self.api_key)
if headers is not None:
return headers

if errors:
logger.debug(
"Headers provider attempts failed: %s",
errors[-1],
exc_info=True,
)
raise AuthenticationError(
message="OpenRouter headers provider failed to produce headers.",
code="missing_credentials",
)
Comment on lines +88 to +138
Copy link

@coderabbitai coderabbitai bot Oct 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Improve error handling to follow coding guidelines.

The provider invocation logic with multiple fallback patterns is well-designed. However, there are coding guideline violations in the error handling:

  1. Line 94: The broad Exception catch violates the guideline that states "If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception." While errors are collected and logged later (Lines 130-134), the intermediate exceptions lose their context.

  2. Lines 100, 108: Error messages don't include details about what the provider returned or which invocation pattern failed, making debugging difficult.

Consider this refactor to improve error context and align with guidelines:

         def _try_provider_call(*args: Any) -> dict[str, str] | None:
             try:
                 result = provider(*args)
             except (AttributeError, TypeError) as exc:
                 errors.append(exc)
                 return None
             except Exception as exc:
-                errors.append(exc)
+                logger.debug(
+                    "Provider call with args %s raised unexpected exception",
+                    args,
+                    exc_info=True
+                )
+                errors.append(
+                    AuthenticationError(
+                        message=f"Provider raised unexpected error: {type(exc).__name__}",
+                        details={"original_error": str(exc), "args": str(args)},
+                    )
+                )
                 return None

             if not isinstance(result, Mapping):
                 errors.append(
-                    TypeError("OpenRouter headers provider must return a mapping."),
+                    TypeError(
+                        f"OpenRouter headers provider must return a mapping, got {type(result).__name__} with args {args}"
+                    ),
                 )
                 return None

             headers = dict(result)
             if not self._authorization_includes_api_key(headers, self.api_key):
                 errors.append(
                     ValueError(
-                        "OpenRouter headers provider did not include API key in Authorization header.",
+                        f"OpenRouter headers provider did not include API key in Authorization header. "
+                        f"Headers: {list(headers.keys())}, Args: {args}"
                     )
                 )
                 return None

             return headers

As per coding guidelines

🏁 Script executed:

#!/bin/bash
set -e

# Locate the definition and exception handling in _try_provider_call
rg -nP "def _try_provider_call" -C3 src/connectors/openrouter.py

# Find broad exception catch
rg -nP "except Exception" -C3 src/connectors/openrouter.py

# Check mapping validation error message
rg -nP 'TypeError\("OpenRouter headers provider must return a mapping' -C1 src/connectors/openrouter.py

# Check debug logging placement
rg -nP "logger.debug" -C3 src/connectors/openrouter.py

Length of output: 1221

Refactor broad exception catch and enrich error context.

  • In _try_provider_call (around line 94), avoid a bare except Exception; log with exc_info=True and wrap or re-raise a specific connector exception per guidelines.
  • Enhance the mapping and authorization error messages to include the actual result type and provider args for clearer debugging.
🧰 Tools
🪛 Ruff (0.13.3)

94-94: Do not catch blind exception: Exception

(BLE001)


def get_headers(self) -> dict[str, str]:
if not self.headers_provider or not self.api_key:
Expand Down
Loading