diff --git a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py index 0989298a8f92..cc60165b34c1 100644 --- a/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py +++ b/components/src/dynamo/frontend/tests/test_vllm_processor_unit.py @@ -1859,6 +1859,17 @@ async def generate(self, preprocessed, **kwargs): # The serialized envelope must not leak to the client. assert "BackendInvalidArgument" not in excinfo.value.message + @pytest.mark.asyncio + async def test_invalid_argument_rejection_propagates(self, vllm_processor_module): + class _RefusingEngine(_FakeRoutedEngine): + async def generate(self, preprocessed, **kwargs): + raise InvalidArgument("use --enable-multimodal") + + processor = _make_processor(vllm_processor_module, _RefusingEngine()) + + with pytest.raises(InvalidArgument, match="--enable-multimodal"): + await _run_generate(processor, _base_preproc()) + @pytest.mark.asyncio async def test_genuine_internal_failure_is_still_internal( self, vllm_processor_module diff --git a/components/src/dynamo/frontend/vllm_processor.py b/components/src/dynamo/frontend/vllm_processor.py index 64514d486d1f..81be99844b74 100644 --- a/components/src/dynamo/frontend/vllm_processor.py +++ b/components/src/dynamo/frontend/vllm_processor.py @@ -41,7 +41,7 @@ from dynamo.common.utils.input_params import resolve_thinking_token_budget from dynamo.frontend.frontend_args import FrontendConfig from dynamo.llm import ModelCardInstanceId, PythonAsyncEngine, RoutedEngine -from dynamo.llm.exceptions import HttpError +from dynamo.llm.exceptions import HttpError, InvalidArgument from dynamo.vllm.errors import vllm_client_error_to_http_error from .prepost import StreamingPostProcessor, preprocess_chat_request @@ -1415,6 +1415,8 @@ async def _generate_and_stream( # translates them at Dynamo's HTTP boundary. The generic handler # below is reserved for genuine internal failures. raise + except InvalidArgument: + raise except Exception as e: backend_error = backend_invalid_argument_to_http_error(e) if backend_error is not None: diff --git a/components/src/dynamo/vllm/handlers.py b/components/src/dynamo/vllm/handlers.py index 99e0ecd2e383..6dc7cba89ed7 100644 --- a/components/src/dynamo/vllm/handlers.py +++ b/components/src/dynamo/vllm/handlers.py @@ -3817,12 +3817,11 @@ async def _assemble_custom_encoder_prompt( # Log with the traceback here — this is the last frame that knows # which request and which encoder — then re-raise unchanged. # - # Deliberately not converted to `InvalidArgument`. The adapters - # raise `ValueError`/`TypeError` for genuine input faults, which the - # bindings already map to `Backend(InvalidArgument)` → 400 carrying - # the message, so the actionable case needs no help. Coercing the - # rest would relabel timeouts, CUDA faults, batcher shutdown and - # cancellations as client errors, suppressing retries — and since + # Deliberately not converted to `InvalidArgument`: adapter + # `ValueError`/`TypeError` map to 400, but with a generic + # body — only `InvalidArgument` carries its message to the client. + # Coercing the rest would relabel timeouts, CUDA faults, batcher shutdown + # and cancellations as client errors, suppressing retries — and since # `encode()` is co-batched, it could blame a caller for a failure # that originated in someone else's request. logger.exception("Request %s: CustomEncoder failed", request_id) diff --git a/lib/bindings/python/rust/errors.rs b/lib/bindings/python/rust/errors.rs index 9fe735d0e7b6..763159167f79 100644 --- a/lib/bindings/python/rust/errors.rs +++ b/lib/bindings/python/rust/errors.rs @@ -161,6 +161,24 @@ fn public_invalid_request_message(error: &DynamoError) -> Option<&str> { } } +fn public_invalid_argument_message(err: &(dyn std::error::Error + 'static)) -> Option { + let mut current = Some(err); + while let Some(error) = current { + if let Some(dynamo_err) = error.downcast_ref::() { + return public_invalid_request_message(dynamo_err).map(str::to_owned); + } + current = error.source(); + } + None +} + +pub fn semantic_to_pyerr(err: &anyhow::Error) -> PyErr { + if let Some(message) = public_invalid_argument_message(err.as_ref()) { + return InvalidArgument::new_err(message); + } + pyo3::exceptions::PyException::new_err(format!("{err}")) +} + /// Read `(code, message)` off a Python exception carrying an HTTP-style /// status. Accepts `.code` (matches [`HttpError`] in `http.rs`) or `.status` /// (matches `dynamo.common.http.HttpStatusError`) plus `.message`. @@ -264,6 +282,35 @@ mod tests { assert_eq!(public_invalid_request_message(&error), None); } + #[derive(Debug)] + struct RouterWrapped(DynamoError); + + impl std::fmt::Display for RouterWrapped { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Prefill execution failed: {}", self.0) + } + } + + impl std::error::Error for RouterWrapped { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.0) + } + } + + #[test] + fn error_chain_yields_public_invalid_argument_message() { + let worker_error = DynamoError::builder() + .class(ErrorClass::InvalidRequest) + .diagnostic("private backend detail") + .public_message("use --enable-multimodal") + .build(); + let wrapped = anyhow::Error::new(RouterWrapped(worker_error)); + assert_eq!( + public_invalid_argument_message(wrapped.as_ref()), + Some("use --enable-multimodal".to_string()) + ); + } + #[test] fn other_error_classes_are_not_promoted_to_invalid_arguments() { for class in [ diff --git a/lib/bindings/python/rust/llm/routed_engine.rs b/lib/bindings/python/rust/llm/routed_engine.rs index 47e228792ece..635a6700d6d3 100644 --- a/lib/bindings/python/rust/llm/routed_engine.rs +++ b/lib/bindings/python/rust/llm/routed_engine.rs @@ -68,7 +68,10 @@ impl RoutedEngine { crate::future_into_py( py, async move { - let mut stream = inner.generate(request_context).await.map_err(to_pyerr)?; + let mut stream = inner + .generate(request_context) + .await + .map_err(|e| crate::errors::semantic_to_pyerr(&e))?; let task_context = stream.context(); let (tx, rx) = tokio::sync::mpsc::channel::>(32); diff --git a/lib/llm/src/kv_router/prefill_router/mod.rs b/lib/llm/src/kv_router/prefill_router/mod.rs index 2569cd7edb93..5cffa80f3afe 100644 --- a/lib/llm/src/kv_router/prefill_router/mod.rs +++ b/lib/llm/src/kv_router/prefill_router/mod.rs @@ -78,9 +78,10 @@ pub enum PrefillError { #[error("Prefill router not yet activated")] NotActivated, - // Callers must include the worker's error text in this message. The - // frontend receives this error through `to_pyerr`, which keeps only - // `Display` and drops the source chain, so a `#[source]` is never seen. + // Callers must include the worker's error text in this message. + // The frontend boundary walks the `#[source]` chain, so a classified + // InvalidRequest with a public message reaches the client with its own + // text. #[error("Prefill execution failed: {0}")] PrefillError( String,