-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
chore(release): backport DB-resilience, passthrough, model-info, budget, and deps fixes to stable/1.88.x #30408
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
6a8e568
dac0f13
c4a7b27
f8c22cd
c58c59e
82a971a
f59192b
44ff751
c86bf9b
91b2013
b083ebe
80f0c38
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 |
|---|---|---|
|
|
@@ -109,6 +109,92 @@ def is_database_transport_error(e: Exception) -> bool: | |
| return True | ||
| return False | ||
|
|
||
| @staticmethod | ||
| def is_prisma_engine_internal_error(e: Exception) -> bool: | ||
| """True iff ``e`` is a non-``PrismaError`` exception raised from inside | ||
| prisma-client-py's query-engine layer. | ||
|
|
||
| During the instant a DB connection is torn down, the query engine can | ||
| return a malformed error payload (``user_facing_error.meta`` is | ||
| ``null``). prisma-client-py's ``handle_response_errors`` then crashes | ||
| with ``AttributeError: 'NoneType' object has no attribute 'get'`` | ||
| before it can raise the proper P1001 "can't reach database server" | ||
| error. That AttributeError carries no connection keyword, so it can't | ||
| be matched by message; identify it by its ``prisma.engine`` origin | ||
| instead. | ||
|
|
||
| Recognized ``PrismaError`` subclasses are excluded: connectivity ones | ||
| are already classified by type/keyword above, and data-layer ones | ||
| (the DB IS reachable) must stay 401. | ||
| """ | ||
| import prisma | ||
|
|
||
| if isinstance(e, prisma.errors.PrismaError): | ||
| return False | ||
| tb = getattr(e, "__traceback__", None) | ||
| while tb is not None: | ||
| if tb.tb_frame.f_globals.get("__name__", "").startswith("prisma.engine"): | ||
| return True | ||
| tb = tb.tb_next | ||
| return False | ||
|
|
||
| @staticmethod | ||
| def is_database_service_unavailable_error(e: Exception) -> bool: | ||
| """True iff the exception means the database could not answer at the | ||
| infrastructure level (connection refused, socket/interface failure, | ||
| timeout) rather than a genuine auth failure (key not found) or a | ||
| data-layer error (the DB IS reachable and rejected the data). | ||
|
|
||
| Auth must answer 401 only for a key the DB confirms is invalid. When | ||
|
Comment on lines
+113
to
+148
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.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| the DB itself is unreachable, the request has to surface as 503 so | ||
| callers retry instead of treating valid keys as invalid during an | ||
| outage. | ||
|
|
||
| Note: prisma-client-py mislabels the P1001 "can't reach database | ||
| server" connectivity failure as a ``DataError`` (a data-layer type), | ||
| so a type-only check misses real outages. ``is_database_transport_error`` | ||
| keyword-matches the connection message and catches that masquerade, | ||
| while genuine data errors (no connection keyword) correctly stay 401. | ||
|
|
||
| The Postgres "cached plan must not change result type" error is matched | ||
|
Comment on lines
+141
to
+159
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.
|
||
| here, not in ``is_database_transport_error``: it is a transient stale-DB- | ||
| state condition (not an invalid key), but the connection is healthy so it | ||
| must not trigger a reconnect. | ||
|
|
||
| A non-``PrismaError`` raised from inside the prisma query engine (e.g. | ||
| the ``AttributeError`` from ``handle_response_errors`` when the engine | ||
| returns a malformed error payload mid-tear-down) is also treated as | ||
| unavailable; see ``is_prisma_engine_internal_error``. | ||
| """ | ||
| import asyncio | ||
|
|
||
| if PrismaDBExceptionHandler.is_database_connection_error(e): | ||
| return True | ||
| if PrismaDBExceptionHandler.is_database_transport_error(e): | ||
| return True | ||
| if PrismaDBExceptionHandler.is_prisma_engine_internal_error(e): | ||
| return True | ||
| if "cached plan must not change result type" in str(e).lower(): | ||
| return True | ||
|
|
||
| # OSError already covers ConnectionError and (Py3.3+) TimeoutError. | ||
| # asyncio.TimeoutError is a distinct class before Py3.11. | ||
| if isinstance(e, (OSError, asyncio.TimeoutError)): | ||
| return True | ||
|
|
||
| try: | ||
| import asyncpg | ||
| except ImportError: | ||
| return False | ||
|
|
||
| return isinstance( | ||
| e, | ||
| ( | ||
| asyncpg.exceptions.PostgresConnectionError, | ||
| asyncpg.exceptions.InterfaceError, | ||
| ), | ||
| ) | ||
|
|
||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| @staticmethod | ||
| def handle_db_exception(e: Exception): | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -100,6 +100,42 @@ def _get_user_from_metadata( | |||||
| return get_end_user_id_from_request_body(request_body) | ||||||
| return None | ||||||
|
|
||||||
| @staticmethod | ||||||
| def _resolve_costing_model(model: str, logging_obj: LiteLLMLoggingObj) -> str: | ||||||
| if model and model != "unknown": | ||||||
| return model | ||||||
| litellm_params = (getattr(logging_obj, "model_call_details", {}) or {}).get( | ||||||
| "litellm_params", {} | ||||||
| ) or {} | ||||||
| deployment_model = litellm_params.get("model") | ||||||
| if deployment_model and deployment_model != "unknown": | ||||||
| return deployment_model | ||||||
| model_group = (litellm_params.get("metadata", {}) or {}).get("model_group") | ||||||
| if model_group: | ||||||
| return model_group.removeprefix("passthrough/") | ||||||
| return model | ||||||
|
|
||||||
| @staticmethod | ||||||
| def _extract_model_from_anthropic_chunks( | ||||||
| all_chunks: Sequence[Union[str, bytes]], | ||||||
| ) -> Optional[str]: | ||||||
| for raw in all_chunks: | ||||||
| text = raw.decode("utf-8") if isinstance(raw, bytes) else raw | ||||||
|
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.
Suggested change
|
||||||
| for line in text.splitlines(): | ||||||
| if not line.startswith("data:"): | ||||||
| continue | ||||||
| try: | ||||||
| data = json.loads(line[len("data:") :].strip()) | ||||||
| except (json.JSONDecodeError, ValueError): | ||||||
| continue | ||||||
| if not isinstance(data, dict): | ||||||
| continue | ||||||
| if data.get("type") == "message_start": | ||||||
| model = (data.get("message") or {}).get("model") | ||||||
| if model: | ||||||
| return model | ||||||
| return None | ||||||
|
|
||||||
| @staticmethod | ||||||
| def _create_anthropic_response_logging_payload( | ||||||
| litellm_model_response: Union[ModelResponse, TextCompletionResponse], | ||||||
|
|
@@ -127,6 +163,10 @@ def _create_anthropic_response_logging_payload( | |||||
| "custom_llm_provider" | ||||||
| ) | ||||||
|
|
||||||
| model = AnthropicPassthroughLoggingHandler._resolve_costing_model( | ||||||
| model, logging_obj | ||||||
| ) | ||||||
|
|
||||||
| # Prepend custom_llm_provider to model if not already present | ||||||
| model_for_cost = model | ||||||
| if custom_llm_provider and not model.startswith(f"{custom_llm_provider}/"): | ||||||
|
|
@@ -213,6 +253,15 @@ def _handle_logging_anthropic_collected_chunks( | |||||
| ): | ||||||
| model = cast(str, litellm_logging_obj.model_call_details.get("model")) | ||||||
|
|
||||||
| if not model or model == "unknown": | ||||||
| chunk_model = ( | ||||||
| AnthropicPassthroughLoggingHandler._extract_model_from_anthropic_chunks( | ||||||
| all_chunks | ||||||
| ) | ||||||
| ) | ||||||
| if chunk_model: | ||||||
| model = chunk_model | ||||||
|
|
||||||
| complete_streaming_response = ( | ||||||
| AnthropicPassthroughLoggingHandler._build_complete_streaming_response( | ||||||
| all_chunks=all_chunks, | ||||||
|
|
@@ -468,6 +517,13 @@ def _build_complete_streaming_response_legacy( | |||||
| # Process each individual event | ||||||
| for event_str in individual_events: | ||||||
| try: | ||||||
| # Skip OpenAI-style [DONE] sentinels some Anthropic-compatible | ||||||
| # providers emit. Match the whole SSE line so a valid chunk whose | ||||||
| # text payload happens to contain "[DONE]" is not dropped. | ||||||
| if any( | ||||||
| line.strip() == "data: [DONE]" for line in event_str.split("\n") | ||||||
| ): | ||||||
| continue | ||||||
| transformed_openai_chunk = anthropic_model_response_iterator.convert_str_chunk_to_generic_chunk( | ||||||
| chunk=event_str | ||||||
| ) | ||||||
|
|
@@ -476,6 +532,14 @@ def _build_complete_streaming_response_legacy( | |||||
|
|
||||||
| except (StopIteration, StopAsyncIteration): | ||||||
| break | ||||||
| except json.JSONDecodeError: | ||||||
| # Some upstreams emit non-JSON SSE lines; skip them so the | ||||||
| # logging pipeline is not broken by a single bad frame. | ||||||
| verbose_proxy_logger.debug( | ||||||
| "Skipping non-JSON SSE event: %s", | ||||||
| event_str[:200], | ||||||
| ) | ||||||
| continue | ||||||
|
|
||||||
| complete_streaming_response = litellm.stream_chunk_builder( | ||||||
| chunks=all_openai_chunks, | ||||||
|
|
||||||
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.
is_prisma_engine_internal_errorwalks every frame ine.__traceback__looking for aprisma.engine.*module name. This works for the specific prisma-client-py version shipped here, but if a future prisma release reorganises its engine modules (e.g., renames toprisma._engine), the check silently stops matching and the malformed-payloadAttributeErrorfalls through to 401 again — exactly the bug this was added to prevent. Worth noting the prisma version assumption in the docstring, and adding a version pin or a canary test against the realprisma.engine.utils.handle_response_errorspath to detect drift early.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!