Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
25 changes: 23 additions & 2 deletions lib/llm/src/http/service/openai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use axum::{
body::Body,
extract::State,
http::Request,
http::{HeaderMap, StatusCode},
http::{HeaderMap, Method, StatusCode, Uri},
middleware::{self, Next},
response::{
IntoResponse, Response,
Expand Down Expand Up @@ -2950,8 +2950,11 @@ pub fn validate_completion_fields_generic(
async fn handler_responses(
State((state, template)): State<(Arc<service_v2::State>, Option<RequestTemplate>)>,
headers: HeaderMap,
Json(mut request): Json<NvCreateResponse>,
body: Bytes,
Comment thread
rmccorm4 marked this conversation as resolved.
Resolved
) -> Result<Response, ErrorResponse> {
ensure_json_content_type(&headers)?;
let mut request: NvCreateResponse = parse_json_request("responses", &body)?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

// return a 503 if the service or model is not ready.
// Resolve the templated model first so empty/missing `model` fields
// don't bypass the gate.
Expand Down Expand Up @@ -3405,6 +3408,24 @@ pub(crate) fn check_ready(state: &Arc<service_v2::State>) -> Result<(), ErrorRes
Ok(())
}

/// Returns an OpenAI-compatible JSON error response for unmatched routes.
/// Axum's default fallback response has an empty body and does not set a
/// `Content-Type` header.
/// This fallback returns `404` with the standard `ErrorMessage` response format.
pub(crate) async fn unmatched_route_fallback(method: Method, uri: Uri) -> ErrorResponse {
Comment thread
rmccorm4 marked this conversation as resolved.
Resolved
let code = StatusCode::NOT_FOUND;
(
code,
Json(ErrorMessage {
message: format!("Invalid URL ({} {})", method, uri.path()),
error_type: map_error_code_to_error_type(code),
code: code.as_u16(),
details: None,
metric_error_type: None,
}),
)
}

/// Canonical, customer-facing message for "model is registered but not yet
/// ready to serve requests" (deployment still initializing or incomplete).
///
Expand Down
118 changes: 118 additions & 0 deletions lib/llm/src/http/service/service_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,10 @@ impl HttpServiceConfigBuilder {

let router = system_router.merge(inference_router);

// Return an OpenAI-compatible JSON error for unmatched routes.
// Configure this after merging routers and before applying layers.
let router = router.fallback(super::openai::unmatched_route_fallback);
Comment thread
pskiran1 marked this conversation as resolved.
Resolved
Comment thread
pskiran1 marked this conversation as resolved.
Resolved

// Echo x-request-id from request to response headers for client correlation
let router = router.layer(axum::middleware::from_fn(echo_request_id_header));

Expand Down Expand Up @@ -1639,6 +1643,120 @@ mod tests {
handle.abort();
}

/// Starts a default `HttpService` on an ephemeral local port.
/// These tests exercise request handling before the readiness check,
/// so no model registration is required.
async fn spawn_default_service() -> (u16, tokio::task::JoinHandle<()>) {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = listener.local_addr().unwrap().port();
let service = HttpService::builder().port(port).build().unwrap();
let handle = tokio::spawn(async move {
service
.run_with_listener(CancellationToken::new(), listener)
.await
.ok();
});
// Allow the server to begin accepting connections.
tokio::time::sleep(Duration::from_millis(20)).await;
(port, handle)
}

/// Verifies that an unsupported content type returns the standard JSON error
/// envelope instead of Axum's default plain-text extractor rejection.
#[tokio::test]
async fn test_responses_non_json_content_type_returns_json_error() {
let (port, handle) = spawn_default_service().await;

let resp = reqwest::Client::new()
.post(format!("http://localhost:{port}/v1/responses"))
.header("content-type", "text/plain")
.body(r#"{"model":"model","input":"hi"}"#)
.send()
.await
.expect("request failed");

assert_eq!(resp.status(), reqwest::StatusCode::UNSUPPORTED_MEDIA_TYPE);
let body: serde_json::Value = resp.json().await.expect("body must be JSON");
assert_eq!(body["code"], 415);
assert_eq!(
body["message"],
"Expected request with Content-Type application/json"
);

handle.abort();
}

/// Verifies that malformed JSON returns the standard JSON error envelope
/// instead of Axum's default plain-text extractor rejection.
#[tokio::test]
async fn test_responses_malformed_json_returns_json_error() {
let (port, handle) = spawn_default_service().await;

let resp = reqwest::Client::new()
.post(format!("http://localhost:{port}/v1/responses"))
.header("content-type", "application/json")
.body(r#"{"model":"model","input":"#)
.send()
.await
.expect("request failed");

assert_eq!(resp.status(), reqwest::StatusCode::BAD_REQUEST);
let body: serde_json::Value = resp.json().await.expect("body must be JSON");
assert_eq!(body["code"], 400);
assert!(
body["message"]
.as_str()
.expect("message must be a string")
.starts_with("Failed to deserialize the JSON body into the target type"),
"unexpected message: {}",
body["message"]
);

handle.abort();
}

/// Verifies that an unknown response ID returns a JSON `404 Not Found`
/// response instead of Axum's default empty fallback response.
#[tokio::test]
async fn test_unknown_response_id_returns_json_404() {
let (port, handle) = spawn_default_service().await;

let resp = reqwest::Client::new()
.get(format!("http://localhost:{port}/v1/responses/resp_missing"))
.send()
.await
.expect("request failed");

assert_eq!(resp.status(), reqwest::StatusCode::NOT_FOUND);
let body: serde_json::Value = resp.json().await.expect("body must be JSON");
assert_eq!(body["code"], 404);
assert_eq!(
body["message"],
"Invalid URL (GET /v1/responses/resp_missing)"
);

handle.abort();
}

/// Verifies that an unmatched route returns the standard JSON `404 Not Found`
/// error envelope.
#[tokio::test]
async fn test_unknown_path_returns_json_404() {
Comment thread
rmccorm4 marked this conversation as resolved.
Resolved
let (port, handle) = spawn_default_service().await;

let resp = reqwest::Client::new()
.get(format!("http://localhost:{port}/v1/not_a_route"))
.send()
.await
.expect("request failed");

assert_eq!(resp.status(), reqwest::StatusCode::NOT_FOUND);
let body: serde_json::Value = resp.json().await.expect("body must be JSON");
assert_eq!(body["code"], 404);

handle.abort();
}

fn make_chat_engine()
-> crate::types::openai::chat_completions::OpenAIChatCompletionsStreamingEngine {
Arc::new(crate::engines::StreamingEngineAdapter::new(
Expand Down
Loading