diff --git a/experimental/sgl-router/monitoring/grafana-dashboard.json b/experimental/sgl-router/monitoring/grafana-dashboard.json index a6d4a78bc..cbef4ac50 100644 --- a/experimental/sgl-router/monitoring/grafana-dashboard.json +++ b/experimental/sgl-router/monitoring/grafana-dashboard.json @@ -511,7 +511,7 @@ { "type": "timeseries", "title": "Responses by HTTP status", - "description": "Client-visible HTTP status codes (2xx/4xx/5xx, 504=stale-cancel).", + "description": "Client-visible HTTP status codes. A 504 is any router-side give-up \u2014 the stale-request deadline or an upstream timeout \u2014 or a worker 504 forwarded unchanged; worker_requests_total{outcome} tells them apart.", "datasource": { "type": "prometheus", "uid": "${datasource}" diff --git a/experimental/sgl-router/src/proxy/mod.rs b/experimental/sgl-router/src/proxy/mod.rs index 5efda106d..405018598 100644 --- a/experimental/sgl-router/src/proxy/mod.rs +++ b/experimental/sgl-router/src/proxy/mod.rs @@ -170,10 +170,15 @@ impl Proxy { let bytes = match resp.bytes().await { Ok(b) => b, Err(e) => { + // Walk the full source chain (`{:#}`) like the connect-error + // handler in `classify_reqwest_error_for` — a mid-body drop's + // real cause (incomplete message, connection reset) lives in the + // wrapped source, not the outer reqwest error. + let cause = anyhow::Error::new(e); tracing::warn!( upstream = %url, status = %status, - error = ?e, + error = %format_args!("{cause:#}"), "upstream dropped connection mid-body", ); breaker.record_failure(); diff --git a/experimental/sgl-router/src/server/error.rs b/experimental/sgl-router/src/server/error.rs index b6445169a..5079b18b8 100644 --- a/experimental/sgl-router/src/server/error.rs +++ b/experimental/sgl-router/src/server/error.rs @@ -9,6 +9,54 @@ use thiserror::Error; pub const X_ROUTER_ERROR_CODE: HeaderName = HeaderName::from_static("x-router-error-code"); +/// Carries the worker's *own* HTTP status when the router had to synthesize a +/// status of its own over a worker that did respond (today: a mid-body drop, +/// where headers arrived but the body did not). Lets a gateway / operator +/// recover what the engine actually said instead of seeing only the router's +/// synthesized 502. Absent on every other response: a forwarded worker response +/// already carries the worker's status in the status line, and a router-only +/// condition (no workers, breaker open, ...) has no upstream status to report. +pub const X_ROUTER_UPSTREAM_STATUS: HeaderName = + HeaderName::from_static("x-router-upstream-status"); + +/// Coarse failure class for a router-originated error. The router's HTTP status +/// is a pure function of the class, so two conditions that mean the same thing +/// — e.g. a per-request timeout and a stale-deadline cancel — can never drift to +/// different status codes. The *precise* condition travels in +/// `x-router-error-code` (see [`ApiError::error_code`]): a gateway in front +/// converts on that header (the authoritative signal), while the status stays a +/// self-sufficient HTTP-honest default for a direct caller. The class never +/// contradicts the precise code — it only generalizes it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ErrorClass { + /// 400 — request rejected at ingress as malformed / invalid. + BadRequest, + /// 404 — requested model / resource not found. + NotFound, + /// 502 — a selected worker failed to return a usable response (unreachable, + /// or started a response then dropped the body). + Upstream, + /// 503 — the router had no worker to dispatch to, or declined to. + NoTarget, + /// 504 — the router gave up waiting (any timeout / deadline). + Timeout, + /// 500 — internal router fault. + Internal, +} + +impl ErrorClass { + fn status(self) -> StatusCode { + match self { + ErrorClass::BadRequest => StatusCode::BAD_REQUEST, + ErrorClass::NotFound => StatusCode::NOT_FOUND, + ErrorClass::Upstream => StatusCode::BAD_GATEWAY, + ErrorClass::NoTarget => StatusCode::SERVICE_UNAVAILABLE, + ErrorClass::Timeout => StatusCode::GATEWAY_TIMEOUT, + ErrorClass::Internal => StatusCode::INTERNAL_SERVER_ERROR, + } + } +} + #[derive(Debug, Error)] pub enum ApiError { #[error("bad request: {0}")] @@ -52,7 +100,7 @@ pub enum ApiError { /// Distinct from `UpstreamUnreachable` (no reply at all) and from a /// well-formed non-2xx (which `Proxy` forwards verbatim with the worker's /// own body). - #[error("upstream returned status {status}")] + #[error("upstream response body incomplete after status {status}")] UpstreamStatus { status: StatusCode }, /// Wall-clock timeout exceeded while waiting for the upstream worker's @@ -89,10 +137,12 @@ pub enum ApiError { /// token wins, the handler returns this variant → HTTP 504 → /// client sees `stale_request_expired`. /// - /// Mapped to 504 (not 503) because the failure is a router-side - /// gateway timeout from the client's perspective: the upstream - /// worker is still potentially fine, the router gave up because - /// the per-request budget elapsed. + /// Classed as [`ErrorClass::Timeout`] (→ 504), the same class as + /// `UpstreamTimeout`: from the client's perspective both are a router-side + /// gateway timeout. Here the upstream worker is still potentially fine — the + /// router gave up because the per-request budget elapsed. The shared class + /// is what keeps the two timeouts on the same status; they stay tellable + /// apart only by `x-router-error-code`. #[error("stale request expired for model {model}")] StaleRequestExpired { model: String }, @@ -126,48 +176,90 @@ pub enum ApiError { } impl ApiError { - fn status_and_code(&self) -> (StatusCode, &'static str) { + /// The failure class — the sole determinant of the HTTP status. Grouping by + /// class is what makes the status non-divergent: every timeout is + /// [`ErrorClass::Timeout`], so a per-request timeout and a stale-deadline + /// cancel are guaranteed the same status, and no future variant can quietly + /// pick a different one. + fn class(&self) -> ErrorClass { match self { - ApiError::BadRequest(_) => (StatusCode::BAD_REQUEST, "bad_request"), - ApiError::SamplingContract { .. } => { - (StatusCode::BAD_REQUEST, "sampling_contract_violation") - } - ApiError::ModelNotFound(_) => (StatusCode::NOT_FOUND, "model_not_found"), - ApiError::UpstreamUnreachable { .. } => { - (StatusCode::BAD_GATEWAY, "upstream_unreachable") - } - ApiError::UpstreamStatus { .. } => (StatusCode::BAD_GATEWAY, "upstream_status"), - ApiError::UpstreamTimeout { .. } => (StatusCode::BAD_GATEWAY, "upstream_timeout"), - ApiError::NoHealthyWorkers { .. } => { - (StatusCode::SERVICE_UNAVAILABLE, "no_healthy_workers") - } - ApiError::NoPrefillWorkersAvailable { .. } => ( - StatusCode::SERVICE_UNAVAILABLE, - "no_prefill_workers_available", - ), - ApiError::NoDecodeWorkersAvailable { .. } => ( - StatusCode::SERVICE_UNAVAILABLE, - "no_decode_workers_available", - ), - ApiError::StaleRequestExpired { .. } => { - (StatusCode::GATEWAY_TIMEOUT, "stale_request_expired") - } - ApiError::PolicySelectionFailed { .. } => { - (StatusCode::SERVICE_UNAVAILABLE, "policy_selection_failed") - } - ApiError::BreakerOpen { .. } => (StatusCode::SERVICE_UNAVAILABLE, "breaker_open"), - ApiError::WorkerMisconfigured { .. } => { - (StatusCode::SERVICE_UNAVAILABLE, "worker_misconfigured") - } - ApiError::Internal(_) => (StatusCode::INTERNAL_SERVER_ERROR, "internal_error"), + ApiError::BadRequest(_) => ErrorClass::BadRequest, + ApiError::SamplingContract { .. } => ErrorClass::BadRequest, + ApiError::ModelNotFound(_) => ErrorClass::NotFound, + ApiError::UpstreamUnreachable { .. } => ErrorClass::Upstream, + ApiError::UpstreamStatus { .. } => ErrorClass::Upstream, + ApiError::UpstreamTimeout { .. } => ErrorClass::Timeout, + ApiError::NoHealthyWorkers { .. } => ErrorClass::NoTarget, + ApiError::NoPrefillWorkersAvailable { .. } => ErrorClass::NoTarget, + ApiError::NoDecodeWorkersAvailable { .. } => ErrorClass::NoTarget, + ApiError::StaleRequestExpired { .. } => ErrorClass::Timeout, + ApiError::PolicySelectionFailed { .. } => ErrorClass::NoTarget, + ApiError::BreakerOpen { .. } => ErrorClass::NoTarget, + ApiError::WorkerMisconfigured { .. } => ErrorClass::NoTarget, + ApiError::Internal(_) => ErrorClass::Internal, } } - /// The HTTP status this error maps to — same value the client receives via - /// `into_response`. Exposed so the access log records the real status - /// (e.g. 502/503/504) instead of a sentinel. + /// Stable, machine-readable `x-router-error-code` — the authoritative signal + /// a gateway converts on. Distinct per condition even when two conditions + /// share a class (and thus a status): `upstream_timeout` and + /// `stale_request_expired` both map to 504 but stay tellable apart here. + fn error_code(&self) -> &'static str { + match self { + ApiError::BadRequest(_) => "bad_request", + // Shares `ErrorClass::BadRequest` (and thus the 400) with + // `BadRequest`, but keeps its own code: an operator rolling a + // sampling contract across a fleet has to be able to alert on + // contract rejections separately from malformed client requests. + ApiError::SamplingContract { .. } => "sampling_contract_violation", + ApiError::ModelNotFound(_) => "model_not_found", + ApiError::UpstreamUnreachable { .. } => "upstream_unreachable", + // Mid-body drop: headers (incl. a status) arrived, then the body + // didn't. We surface a 502 but echo the worker's status in + // `x-router-upstream-status` (see `into_response`). + ApiError::UpstreamStatus { .. } => "upstream_body_incomplete", + ApiError::UpstreamTimeout { .. } => "upstream_timeout", + ApiError::NoHealthyWorkers { .. } => "no_healthy_workers", + ApiError::NoPrefillWorkersAvailable { .. } => "no_prefill_workers_available", + ApiError::NoDecodeWorkersAvailable { .. } => "no_decode_workers_available", + ApiError::StaleRequestExpired { .. } => "stale_request_expired", + ApiError::PolicySelectionFailed { .. } => "policy_selection_failed", + ApiError::BreakerOpen { .. } => "breaker_open", + ApiError::WorkerMisconfigured { .. } => "worker_misconfigured", + ApiError::Internal(_) => "internal_error", + } + } + + /// The worker's *own* status to echo in `x-router-upstream-status`, for the + /// case where the router synthesized its own status over a worker that did + /// respond. Today only the mid-body-drop (`UpstreamStatus`) carries one. This + /// is an exhaustive, wildcard-free match (not an `if let` at the call site) so + /// a future "synthesized over a responding worker" variant is forced to decide + /// whether it echoes a status, rather than silently inheriting `None`. + fn upstream_status(&self) -> Option { + match self { + ApiError::UpstreamStatus { status } => Some(*status), + ApiError::BadRequest(_) + | ApiError::SamplingContract { .. } + | ApiError::ModelNotFound(_) + | ApiError::UpstreamUnreachable { .. } + | ApiError::UpstreamTimeout { .. } + | ApiError::NoHealthyWorkers { .. } + | ApiError::NoPrefillWorkersAvailable { .. } + | ApiError::NoDecodeWorkersAvailable { .. } + | ApiError::StaleRequestExpired { .. } + | ApiError::PolicySelectionFailed { .. } + | ApiError::BreakerOpen { .. } + | ApiError::WorkerMisconfigured { .. } + | ApiError::Internal(_) => None, + } + } + + /// The HTTP status this error maps to — the same value the client receives + /// via `into_response`. Exposed so a caller holding the error, rather than + /// the response, can label it with the status the client actually saw. pub fn status_code(&self) -> StatusCode { - self.status_and_code().0 + self.class().status() } } @@ -186,7 +278,8 @@ struct ErrorBody<'a> { impl IntoResponse for ApiError { fn into_response(self) -> Response { - let (status, code) = self.status_and_code(); + let status = self.class().status(); + let code = self.error_code(); let typ = match status.as_u16() { 400..=499 => "invalid_request_error", _ => "server_error", @@ -209,11 +302,14 @@ impl IntoResponse for ApiError { "upstream unavailable".to_string() } ApiError::UpstreamStatus { status } => { + // The worker's status is usually 200 here — it answered, then + // dropped the body — so neither message may call it an error + // status. tracing::warn!( upstream_status = %status, - "upstream returned an error status", + "upstream response body did not complete", ); - "upstream returned an error status".to_string() + "upstream response was incomplete".to_string() } ApiError::UpstreamTimeout { worker } => { tracing::warn!(upstream = %worker, "upstream request timed out"); @@ -273,6 +369,15 @@ impl IntoResponse for ApiError { .into_response(); resp.headers_mut() .insert(X_ROUTER_ERROR_CODE, HeaderValue::from_static(code)); + // Preserve the worker's real status when we synthesized our own (today, + // only the mid-body-drop case: the worker sent a status, then dropped the + // body, so we report a 502 but don't throw away what it said). + if let Some(upstream) = self.upstream_status() { + resp.headers_mut().insert( + X_ROUTER_UPSTREAM_STATUS, + HeaderValue::from(upstream.as_u16()), + ); + } resp } } @@ -348,7 +453,10 @@ mod tests { } #[test] - fn upstream_status_envelope_has_code() { + fn upstream_body_incomplete_preserves_worker_status_in_header() { + // Mid-body drop: the worker sent a status (here 500) then dropped the + // body. The router synthesizes its own 502, but the worker's real status + // is preserved in `x-router-upstream-status` rather than silently lost. let err = ApiError::UpstreamStatus { status: StatusCode::INTERNAL_SERVER_ERROR, }; @@ -358,10 +466,20 @@ mod tests { resp.headers() .get("x-router-error-code") .and_then(|v| v.to_str().ok()), - Some("upstream_status"), + Some("upstream_body_incomplete"), + ); + assert_eq!( + resp.headers() + .get("x-router-upstream-status") + .and_then(|v| v.to_str().ok()), + Some("500"), + "the worker's real status must be preserved, not discarded", ); let body = collect_body(resp); - assert!(body.contains("\"code\":\"upstream_status\""), "{body}"); + assert!( + body.contains("\"code\":\"upstream_body_incomplete\""), + "{body}" + ); } #[test] @@ -372,7 +490,9 @@ mod tests { worker: worker.clone(), }; let resp = err.into_response(); - assert_eq!(resp.status(), StatusCode::BAD_GATEWAY); + // A timeout is a gateway timeout (504), not a bad gateway (502): the + // same class — and so the same status — as the stale-deadline cancel. + assert_eq!(resp.status(), StatusCode::GATEWAY_TIMEOUT); assert_eq!( resp.headers() .get("x-router-error-code") @@ -485,4 +605,165 @@ mod tests { env.error.message ); } + + /// The three client-facing signals of the router status-code contract: + /// the HTTP status, `x-router-error-code`, and `x-router-upstream-status`. + fn signals(err: ApiError) -> (StatusCode, Option, Option) { + let resp = err.into_response(); + let status = resp.status(); + let header = |name: &str| { + resp.headers() + .get(name) + .and_then(|v| v.to_str().ok()) + .map(str::to_owned) + }; + ( + status, + header("x-router-error-code"), + header("x-router-upstream-status"), + ) + } + + /// Every router-*originated* condition maps to a fixed (status, error-code) + /// pair, and ONLY the mid-body-drop case echoes the worker's real status in + /// `x-router-upstream-status`. This pins the router half of the status-code + /// contract: + /// * same condition class → same status — both timeouts are 504, so the + /// per-request timeout and the stale-deadline cancel cannot diverge; + /// * a worker's real status is preserved, never silently rewritten. + #[test] + fn router_originated_scenarios_match_status_and_headers() { + let worker = reqwest::Url::parse("http://host:1/").unwrap(); + // (label, error, expected status, expected x-router-error-code, + // expected x-router-upstream-status) + let cases: Vec<(&str, ApiError, StatusCode, &str, Option<&str>)> = vec![ + ( + "mid-body drop preserves worker status", + ApiError::UpstreamStatus { + status: StatusCode::OK, + }, + StatusCode::BAD_GATEWAY, + "upstream_body_incomplete", + Some("200"), + ), + ( + "unreachable", + ApiError::UpstreamUnreachable { + worker: worker.clone(), + source: anyhow::anyhow!("connect refused"), + }, + StatusCode::BAD_GATEWAY, + "upstream_unreachable", + None, + ), + ( + "request timeout", + ApiError::UpstreamTimeout { + worker: worker.clone(), + }, + StatusCode::GATEWAY_TIMEOUT, + "upstream_timeout", + None, + ), + ( + "stale-deadline cancel", + ApiError::StaleRequestExpired { model: "m".into() }, + StatusCode::GATEWAY_TIMEOUT, + "stale_request_expired", + None, + ), + ( + "no healthy workers", + ApiError::NoHealthyWorkers { model: "m".into() }, + StatusCode::SERVICE_UNAVAILABLE, + "no_healthy_workers", + None, + ), + ( + "bad request", + ApiError::BadRequest("bad".into()), + StatusCode::BAD_REQUEST, + "bad_request", + None, + ), + ( + "sampling contract rejection", + ApiError::SamplingContract { + param: "temperature", + detail: "got 0.5, expected 1".into(), + }, + StatusCode::BAD_REQUEST, + "sampling_contract_violation", + None, + ), + ( + "model not found", + ApiError::ModelNotFound("ghost".into()), + StatusCode::NOT_FOUND, + "model_not_found", + None, + ), + ( + "no prefill workers", + ApiError::NoPrefillWorkersAvailable { model: "m".into() }, + StatusCode::SERVICE_UNAVAILABLE, + "no_prefill_workers_available", + None, + ), + ( + "no decode workers", + ApiError::NoDecodeWorkersAvailable { model: "m".into() }, + StatusCode::SERVICE_UNAVAILABLE, + "no_decode_workers_available", + None, + ), + ( + "policy selection failed", + ApiError::PolicySelectionFailed { model: "m".into() }, + StatusCode::SERVICE_UNAVAILABLE, + "policy_selection_failed", + None, + ), + ( + "breaker open", + ApiError::BreakerOpen { + worker: "http://w:1".into(), + }, + StatusCode::SERVICE_UNAVAILABLE, + "breaker_open", + None, + ), + ( + "worker misconfigured", + ApiError::WorkerMisconfigured { + worker: "http://w:1".into(), + source: anyhow::anyhow!("unparsable url"), + }, + StatusCode::SERVICE_UNAVAILABLE, + "worker_misconfigured", + None, + ), + ( + "internal", + ApiError::Internal(anyhow::anyhow!("boom")), + StatusCode::INTERNAL_SERVER_ERROR, + "internal_error", + None, + ), + ]; + for (label, err, want_status, want_code, want_upstream) in cases { + let (status, code, upstream) = signals(err); + assert_eq!(status, want_status, "status mismatch for `{label}`"); + assert_eq!( + code.as_deref(), + Some(want_code), + "x-router-error-code mismatch for `{label}`", + ); + assert_eq!( + upstream.as_deref(), + want_upstream, + "x-router-upstream-status mismatch for `{label}`", + ); + } + } } diff --git a/experimental/sgl-router/tests/proxy/chat_routing.rs b/experimental/sgl-router/tests/proxy/chat_routing.rs index 7adb26f9f..5b88f625b 100644 --- a/experimental/sgl-router/tests/proxy/chat_routing.rs +++ b/experimental/sgl-router/tests/proxy/chat_routing.rs @@ -467,11 +467,17 @@ async fn non_streaming_upstream_429_preserved() { res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json", ); - // Router envelope code header must NOT be set — this is upstream's response. + // Router envelope headers must NOT be set — this is upstream's response. + // Their absence is exactly how a gateway tells "the engine said this" from + // "the router said this". assert!( res.headers().get("x-router-error-code").is_none(), "router envelope header must NOT be set on upstream-passthrough responses", ); + assert!( + res.headers().get("x-router-upstream-status").is_none(), + "no status was synthesized over the worker, so no x-router-upstream-status", + ); let bytes = res.into_body().collect().await.unwrap().to_bytes(); let got: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); assert_eq!(got, upstream_body, "body bytes must round-trip unchanged"); @@ -511,6 +517,11 @@ async fn non_streaming_upstream_500_preserved() { res.headers().get("x-router-error-code").is_none(), "router envelope must NOT wrap upstream 5xx — passthrough", ); + assert!( + res.headers().get("x-router-upstream-status").is_none(), + "a complete worker 500 is forwarded verbatim — distinct from a \ + synthesized 502 mid-body drop, which DOES echo the worker status", + ); let bytes = res.into_body().collect().await.unwrap().to_bytes(); let got: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); assert_eq!(got, upstream_body); @@ -670,12 +681,14 @@ async fn chat_rejects_string_body_400() { } #[tokio::test] -async fn non_streaming_mid_body_drop_classified_as_upstream_status() { +async fn non_streaming_mid_body_drop_classified_as_upstream_body_incomplete() { // Regression: when the upstream replies with a status line and headers // but drops the connection mid-body, the failure is NOT // "upstream_unreachable" (the upstream demonstrably DID reply). It must - // be classified as `upstream_status` so the operator-visible envelope - // reflects that the worker partially served the request. + // be classified as `upstream_body_incomplete` so the operator-visible + // envelope reflects that the worker partially served the request — and the + // worker's own status must survive in `x-router-upstream-status` rather + // than being replaced by the router's synthesized 502. let worker = crate::common::mock_worker::MockWorker::start_returning_partial_body( StatusCode::OK, b"{\"partial\": ", @@ -705,8 +718,13 @@ async fn non_streaming_mid_body_drop_classified_as_upstream_status() { ); assert_eq!( res.headers().get("x-router-error-code").unwrap(), - "upstream_status", - "mid-body drop must be upstream_status (worker DID reply), not upstream_unreachable", + "upstream_body_incomplete", + "mid-body drop must be upstream_body_incomplete (worker DID reply), not upstream_unreachable", + ); + assert_eq!( + res.headers().get("x-router-upstream-status").unwrap(), + "200", + "the worker's own status must be echoed, not discarded behind the 502", ); } @@ -976,6 +994,61 @@ async fn forward_streaming_to_records_failure_on_mid_stream_drop() { ); } +/// Client-visible contract of a streaming mid-body drop, through `build_router`. +/// This is the asymmetric half of the non-streaming case: headers were already +/// sent as 200, so the client keeps a 200 (NOT the synthesized 502 of the +/// non-streaming path), there is NO `x-router-error-code` / +/// `x-router-upstream-status`, and `responses_total` counts it as a 200 (the +/// breaker / duration metrics capture the mid-stream failure — see the breaker +/// test above). +#[tokio::test] +async fn streaming_mid_body_drop_stays_200_with_no_router_headers() { + let worker = crate::common::mock_worker::MockWorker::start_returning_partial_body( + StatusCode::OK, + b"data: hi\n\n", + ) + .await; + let ctx = build_ctx_with_worker(&worker.url); + let app = build_router(ctx.clone()); + + let req = Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("content-type", "application/json") + .body(Body::from( + serde_json::to_vec(&serde_json::json!({ + "model": "tiny", + "messages": [{"role": "user", "content": "hi"}], + "stream": true + })) + .unwrap(), + )) + .unwrap(); + let res = app.oneshot(req).await.unwrap(); + assert_eq!( + res.status(), + StatusCode::OK, + "streaming mid-drop: headers were already sent as 200, so the client keeps 200", + ); + assert!( + res.headers().get("x-router-error-code").is_none(), + "a 200-then-drop stream is not router-originated — no x-router-error-code", + ); + assert!( + res.headers().get("x-router-upstream-status").is_none(), + "no status was synthesized over the worker, so no x-router-upstream-status", + ); + let _ = res.into_body().collect().await; + + let m = ctx.metrics.render(); + assert!( + m.contains( + r#"sgl_router_responses_total{route="/v1/chat/completions",method="POST",status_code="200"} 1"# + ), + "a streaming mid-drop counts as a 200 at the edge: {m}", + ); +} + #[tokio::test] async fn forward_json_to_records_failure_on_5xx() { use sgl_router::health::circuit_breaker::{CircuitBreaker, CircuitBreakerConfig}; diff --git a/experimental/sgl-router/tests/proxy/timeout.rs b/experimental/sgl-router/tests/proxy/timeout.rs index c64edd02c..5ea4cdc5c 100644 --- a/experimental/sgl-router/tests/proxy/timeout.rs +++ b/experimental/sgl-router/tests/proxy/timeout.rs @@ -7,7 +7,8 @@ //! Without a configured `.timeout(...)` on the reqwest client, a stalled //! backend hangs the axum handler future forever and the test harness //! would just timeout. We assert here that the router returns a fast, -//! clean 502 (`upstream_timeout`) instead. +//! clean 504 (`upstream_timeout`) instead — a timeout is a gateway timeout, +//! the same status class as the stale-deadline cancel. use axum::body::Body; use axum::http::{Request, StatusCode}; @@ -103,7 +104,7 @@ async fn non_streaming_request_times_out_when_worker_hangs() { elapsed < Duration::from_secs(1), "router must short-circuit on upstream timeout; elapsed {elapsed:?}" ); - assert_eq!(res.status(), StatusCode::BAD_GATEWAY); + assert_eq!(res.status(), StatusCode::GATEWAY_TIMEOUT); assert_eq!( res.headers().get("x-router-error-code").unwrap(), "upstream_timeout"