From a2efe1eeb3a4e71715bc77528d20bd52340203f2 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 25 May 2026 18:39:44 -0400 Subject: [PATCH] let's go with this... --- Cargo.lock | 1 + Cargo.toml | 1 + crates/service/web/storyteller_web/Cargo.toml | 1 + .../common_responses/common_web_error.rs | 75 ++++++++++++++----- .../endpoints/dev/dev_input_error_handler.rs | 2 +- 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cb91f9243a..861295d67e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11542,6 +11542,7 @@ dependencies = [ "elasticsearch", "elasticsearch_schema", "enums", + "erased-serde", "error_utils", "errors", "fal_client", diff --git a/Cargo.toml b/Cargo.toml index 41d44ce5fe..43222a92b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -334,6 +334,7 @@ regex = "1.12.3" reqwest = { version = "0.12.23", default-features = false, features = [ "charset", "http2", "system-proxy", "json", "cookies", "rustls-tls", "gzip" ] } reqwest-websocket = { version = "0.5.0" } rust-s3 = { version = "0.36.0-beta.2", default-features = false, features = ["fail-on-err", "tags", "tokio-rustls-tls"] } +erased-serde = "0.4" serde = { version = "1.0.228", features = ["derive"] } serde_derive = "1.0.228" serde_json = "1.0.149" diff --git a/crates/service/web/storyteller_web/Cargo.toml b/crates/service/web/storyteller_web/Cargo.toml index 1bf4556c56..b1d6ca70f6 100644 --- a/crates/service/web/storyteller_web/Cargo.toml +++ b/crates/service/web/storyteller_web/Cargo.toml @@ -175,6 +175,7 @@ magic-crypt = "3.1.13" # encrypting sort IDs for the frontend so we don't leak i md-5 = "0.10.6" # gravatar # Serialization +erased-serde.workspace = true serde.workspace = true serde_derive.workspace = true serde_json.workspace = true diff --git a/crates/service/web/storyteller_web/src/http_server/common_responses/common_web_error.rs b/crates/service/web/storyteller_web/src/http_server/common_responses/common_web_error.rs index c66b384206..29163367ad 100644 --- a/crates/service/web/storyteller_web/src/http_server/common_responses/common_web_error.rs +++ b/crates/service/web/storyteller_web/src/http_server/common_responses/common_web_error.rs @@ -39,9 +39,16 @@ pub enum CommonWebError { BadInputWithSimpleMessage(String), /// 400 Bad Request with an endpoint-specific JSON body. - /// The body is whatever the caller serialized (e.g. a per-field validation - /// failure list). Construct via [`CommonWebError::bad_input_tailored_response`]. - BadInputTailoredResponse(serde_json::Value), + /// The caller hands over any `Serialize` struct; the middleware writes it + /// directly as the response body with `Content-Type: application/json`. + /// Construct via [`CommonWebError::bad_input_tailored_response`]. + /// + /// Stored as `Arc`: + /// `serde::Serialize` isn't object-safe (its method is generic over + /// `Serializer`), so we use `erased_serde::Serialize` — a sibling trait + /// with a non-generic method, auto-implemented for every `T: Serialize`. + /// `Arc` (instead of `Box`) keeps the enum `Clone`. + BadInputTailoredResponse(Arc), /// 401 Unauthorized. NotAuthorized, @@ -106,16 +113,18 @@ impl CommonWebError { /// /// Use this when an endpoint needs to return a structured validation payload /// (per-field errors, etc.) rather than the generic `{success, error_code, message}` - /// envelope used by [`BadInputWithSimpleMessage`]. + /// envelope used by [`BadInputWithSimpleMessage`]. The caller passes its own + /// `Serialize` type directly — no envelope, no `serde_json::Value` boxing + /// at the callsite. + /// Build a 400 Bad Request whose body is the JSON-serialized form of `body`. /// - /// If serialization fails (rare — usually only when a `Map` has non-string keys), - /// falls back to an [`UncaughtServerError`] so the failure is paged and the - /// caller never sees a corrupt response. - pub fn bad_input_tailored_response(body: &T) -> Self { - match serde_json::to_value(body) { - Ok(value) => Self::BadInputTailoredResponse(value), - Err(err) => Self::from_error(err), - } + /// The caller passes its typed struct directly. Serialization is deferred to + /// response time, when the middleware writes the JSON straight onto the wire. + pub fn bad_input_tailored_response(body: T) -> Self + where + T: Serialize + Send + Sync + 'static, + { + Self::BadInputTailoredResponse(Arc::new(body)) } /// Extract the wrapped causal error (only present for `UncaughtServerError`). @@ -153,7 +162,12 @@ impl Display for CommonWebError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Self::BadInputWithSimpleMessage(msg) => write!(f, "Bad input: {}", msg), - Self::BadInputTailoredResponse(value) => write!(f, "Bad input (tailored): {}", value), + Self::BadInputTailoredResponse(payload) => { + match serde_json::to_string(payload) { + Ok(s) => write!(f, "Bad input (tailored): {}", s), + Err(_) => write!(f, "Bad input (tailored): "), + } + } Self::NotAuthorized => write!(f, "Not authorized"), Self::NotFound => write!(f, "Not found"), Self::PaymentRequired => write!(f, "Payment required"), @@ -172,7 +186,12 @@ impl std::fmt::Debug for CommonWebError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Self::BadInputWithSimpleMessage(msg) => write!(f, "BadInputWithSimpleMessage({:?})", msg), - Self::BadInputTailoredResponse(value) => write!(f, "BadInputTailoredResponse({})", value), + Self::BadInputTailoredResponse(payload) => { + match serde_json::to_string(payload) { + Ok(s) => write!(f, "BadInputTailoredResponse({})", s), + Err(_) => write!(f, "BadInputTailoredResponse()"), + } + } Self::NotAuthorized => write!(f, "NotAuthorized"), Self::NotFound => write!(f, "NotFound"), Self::PaymentRequired => write!(f, "PaymentRequired"), @@ -228,10 +247,28 @@ impl ResponseError for CommonWebError { message: msg, }) } - Self::BadInputTailoredResponse(value) => { - // The body is whatever the caller serialized — no envelope. - // `.json(...)` sets Content-Type: application/json automatically. - HttpResponse::BadRequest().json(value) + Self::BadInputTailoredResponse(payload) => { + // Body is the caller-supplied serializable struct — no envelope. + // `serde_json::to_vec` accepts `&dyn erased_serde::Serialize`. + match serde_json::to_vec(payload) { + Ok(bytes) => { + HttpResponseBuilder::new(status) + .insert_header(("Content-Type", "application/json")) + .body(bytes) + } + Err(err) => { + // Serialization failed (rare — non-string Map keys, etc.). + // Fall back to a generic 500 so the failure is observable + // rather than producing a corrupt 400 body. + log::error!("BadInputTailoredResponse serialization failed: {:?}", err); + HttpResponseBuilder::new(StatusCode::INTERNAL_SERVER_ERROR) + .json(JsonErrorWithoutMessage { + success: false, + error_code: 500, + error_code_str: Some("Internal Server Error"), + }) + } + } } Self::ContentPolicyRejected => { HttpResponseBuilder::new(status) @@ -425,7 +462,7 @@ mod tests { count: u32, } - let error = CommonWebError::bad_input_tailored_response(&Sample { + let error = CommonWebError::bad_input_tailored_response(Sample { reason: "no good", count: 3, }); diff --git a/crates/service/web/storyteller_web/src/http_server/endpoints/dev/dev_input_error_handler.rs b/crates/service/web/storyteller_web/src/http_server/endpoints/dev/dev_input_error_handler.rs index ccdfd89ff0..7667a0e04b 100644 --- a/crates/service/web/storyteller_web/src/http_server/endpoints/dev/dev_input_error_handler.rs +++ b/crates/service/web/storyteller_web/src/http_server/endpoints/dev/dev_input_error_handler.rs @@ -38,7 +38,7 @@ pub async fn dev_input_error_handler() -> Result { ], }; - Err(CommonWebError::bad_input_tailored_response(&body)) + Err(CommonWebError::bad_input_tailored_response(body)) } #[derive(Serialize)]