-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
fix(proxy): make async_post_call_response_headers_hook consistent across all endpoints #22985
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.
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
Changes from all commits
e7abad1
e17c1ce
5fc852a
9eb8b98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||
| import copy | ||||||||||||||||||||||
| import hashlib | ||||||||||||||||||||||
| import inspect | ||||||||||||||||||||||
| import json | ||||||||||||||||||||||
| import os | ||||||||||||||||||||||
| import smtplib | ||||||||||||||||||||||
|
|
@@ -285,6 +286,19 @@ def get_cache( | |||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### LOGGING ### | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Cache for inspect.signature checks — avoids repeated introspection per request | ||||||||||||||||||||||
| _CALLBACK_ACCEPTS_CALL_INFO: Dict[int, bool] = {} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def _accepts_litellm_call_info(cb: CustomLogger) -> bool: | ||||||||||||||||||||||
| key = id(type(cb)) | ||||||||||||||||||||||
| if key not in _CALLBACK_ACCEPTS_CALL_INFO: | ||||||||||||||||||||||
| sig = inspect.signature(cb.async_post_call_response_headers_hook) | ||||||||||||||||||||||
| _CALLBACK_ACCEPTS_CALL_INFO[key] = "litellm_call_info" in sig.parameters | ||||||||||||||||||||||
| return _CALLBACK_ACCEPTS_CALL_INFO[key] | ||||||||||||||||||||||
|
Comment on lines
+294
to
+299
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. VAR_KEYWORD callbacks silently excluded from
The fix is to also accept callbacks whose signature contains a sig = inspect.signature(cb.async_post_call_response_headers_hook)
has_explicit = "litellm_call_info" in sig.parameters
has_var_kw = any(
p.kind == inspect.Parameter.VAR_KEYWORD
for p in sig.parameters.values()
)
_CALLBACK_ACCEPTS_CALL_INFO[key] = has_explicit or has_var_kw |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| class ProxyLogging: | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| Logging/Custom Handlers for proxy. | ||||||||||||||||||||||
|
|
@@ -1975,6 +1989,9 @@ async def post_call_response_headers_hook( | |||||||||||||||||||||
| """ | ||||||||||||||||||||||
| merged_headers: Dict[str, str] = {} | ||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| # Build litellm_call_info — normalized routing metadata for callbacks | ||||||||||||||||||||||
| litellm_call_info = self._build_litellm_call_info(data=data, response=response) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for callback in litellm.callbacks: | ||||||||||||||||||||||
| _callback: Optional[CustomLogger] = None | ||||||||||||||||||||||
| if isinstance(callback, str): | ||||||||||||||||||||||
|
|
@@ -1985,12 +2002,22 @@ async def post_call_response_headers_hook( | |||||||||||||||||||||
| _callback = callback # type: ignore | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if _callback is not None and isinstance(_callback, CustomLogger): | ||||||||||||||||||||||
| result = await _callback.async_post_call_response_headers_hook( | ||||||||||||||||||||||
| data=data, | ||||||||||||||||||||||
| user_api_key_dict=user_api_key_dict, | ||||||||||||||||||||||
| response=response, | ||||||||||||||||||||||
| request_headers=request_headers, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| if _accepts_litellm_call_info(_callback): | ||||||||||||||||||||||
| result = await _callback.async_post_call_response_headers_hook( | ||||||||||||||||||||||
| data=data, | ||||||||||||||||||||||
| user_api_key_dict=user_api_key_dict, | ||||||||||||||||||||||
| response=response, | ||||||||||||||||||||||
| request_headers=request_headers, | ||||||||||||||||||||||
| litellm_call_info=litellm_call_info, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| else: | ||||||||||||||||||||||
| # Backwards compat: callback doesn't accept litellm_call_info | ||||||||||||||||||||||
| result = await _callback.async_post_call_response_headers_hook( | ||||||||||||||||||||||
| data=data, | ||||||||||||||||||||||
| user_api_key_dict=user_api_key_dict, | ||||||||||||||||||||||
| response=response, | ||||||||||||||||||||||
| request_headers=request_headers, | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| if result is not None: | ||||||||||||||||||||||
| merged_headers.update(result) | ||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||
|
|
@@ -1999,6 +2026,30 @@ async def post_call_response_headers_hook( | |||||||||||||||||||||
| ) | ||||||||||||||||||||||
| return merged_headers | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||||
| def _build_litellm_call_info( | ||||||||||||||||||||||
| data: dict, response: Any | ||||||||||||||||||||||
| ) -> Dict[str, Any]: | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| Build a normalized dict of routing metadata from response._hidden_params | ||||||||||||||||||||||
| and data, abstracting away the metadata vs litellm_metadata split. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| hidden_params = getattr(response, "_hidden_params", {}) or {} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # model_info: check both metadata keys (chat uses "metadata", responses uses "litellm_metadata") | ||||||||||||||||||||||
| model_info = ( | ||||||||||||||||||||||
| (data.get("metadata") or {}).get("model_info") | ||||||||||||||||||||||
| or (data.get("litellm_metadata") or {}).get("model_info") | ||||||||||||||||||||||
| or {} | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
Comment on lines
+2040
to
+2044
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The failure path in
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return { | ||||||||||||||||||||||
| "custom_llm_provider": hidden_params.get("custom_llm_provider"), | ||||||||||||||||||||||
| "model_info": model_info, | ||||||||||||||||||||||
| "api_base": hidden_params.get("api_base"), | ||||||||||||||||||||||
| "model_id": hidden_params.get("model_id"), | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def is_a2a_streaming_response(self, response: dict) -> bool: | ||||||||||||||||||||||
| expected_keys = ["jsonrpc", "id", "result"] | ||||||||||||||||||||||
| return all(key in response for key in expected_keys) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache key uses
id(cb)(instance memory address), which is vulnerable to Python's address reuse after garbage collection. In deployments with dynamic callback add/remove: a removed callback's address may be reused by a new callback that doesn't accept litellm_call_info, causing the stale cache entry to return the wrong signature status.Since all instances of the same callback class share the same method signature, use
id(type(cb))instead. This is safer (classes are effectively singletons), has better cache hit rates, and avoids the GC reuse problem entirely.