From c67ef194a9b5942fdf0570f6ee299c8abaf425e3 Mon Sep 17 00:00:00 2001 From: luxamior Date: Sun, 23 Aug 2026 18:18:14 +0900 Subject: [PATCH] Explain schema mismatches as a version skew `map_api_error_structured` had no arm for progenitor's `InvalidResponsePayload`, so it fell through to the `other` branch and was rendered by progenitor's own Display: Error::InvalidResponsePayload(b, e) => write!(f, "Invalid Response Payload ({:?}): {}", b, e) That `{:?}` dumps the entire body. Running a 0.254 CLI against a 0.333 server turned `fabro inspect` into 30KB of escaped JSON with no statement of the cause, and `fabro events` did the same. `fabro version` and `fabro doctor` both report the mismatch correctly; the commands that actually fail did not. Give the variant its own arm. The message now leads with the schema mismatch, names the CLI version, and points at `fabro version` to compare with the server followed by `fabro upgrade`. It mentions `--prerelease` because the plain upgrade path only considers stable releases, so a server on a nightly leaves the CLI reporting it is already current. The body is kept as a 200-character preview, enough to recognize the payload without scrolling the remediation away. `classify_api_error` delegates to `map_api_error_structured`, so it picks this up too. Verified with cargo check, cargo test (9 passed, including the 7 that already existed), and cargo clippy --all-targets -- -D warnings, all on stable 1.98.0 in Docker. The pinned nightly-2026-04-14 fmt and clippy runs that CI uses have not been run. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HMb7GgUkEkk8CWc6MPpy2w --- lib/foundation/fabro-client/src/error.rs | 77 +++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/lib/foundation/fabro-client/src/error.rs b/lib/foundation/fabro-client/src/error.rs index b5b2c9a07..71f4a2a4a 100644 --- a/lib/foundation/fabro-client/src/error.rs +++ b/lib/foundation/fabro-client/src/error.rs @@ -1,5 +1,6 @@ use anyhow::{Result, anyhow}; use fabro_util::exit::{ErrorExt, ExitClass}; +use fabro_util::version::FABRO_VERSION; use serde::de::DeserializeOwned; #[derive(Debug, Clone, PartialEq, Eq)] @@ -150,6 +151,10 @@ where let status = response.status(); build_structured_error(anyhow!("request failed with status {status}"), status, None) } + progenitor_client::Error::InvalidResponsePayload(body, source) => StructuredApiError { + error: schema_mismatch_error(&body, &source), + failure: None, + }, other => StructuredApiError { error: anyhow::Error::new(other), failure: None, @@ -157,6 +162,34 @@ where } } +/// Maximum number of body characters echoed back when a response fails to +/// deserialize. Enough to recognize the payload, short enough that the +/// remediation hint is not scrolled off the terminal. +const SCHEMA_MISMATCH_BODY_PREVIEW: usize = 200; + +/// The server answered successfully but its payload does not fit the schema +/// this CLI was generated against. In practice that nearly always means the +/// two are on different versions, so lead with that rather than dumping the +/// whole body and leaving the reader to guess. +fn schema_mismatch_error(body: &[u8], source: &serde_json::Error) -> anyhow::Error { + let text = String::from_utf8_lossy(body); + let preview: String = text.chars().take(SCHEMA_MISMATCH_BODY_PREVIEW).collect(); + let ellipsis = if text.chars().nth(SCHEMA_MISMATCH_BODY_PREVIEW).is_some() { + "..." + } else { + "" + }; + + anyhow!( + "server response did not match the schema this CLI expects: {source}\n\n\ + This usually means the CLI and the server are running different versions. \ + This CLI is {FABRO_VERSION}; run `fabro version` to compare it with the server, \ + then upgrade the older side with `fabro upgrade` (add `--prerelease` if the \ + server runs a nightly build).\n\n\ + Response body started with: {preview}{ellipsis}" + ) +} + pub fn map_api_error(err: progenitor_client::Error) -> anyhow::Error where E: serde::Serialize + std::fmt::Debug + Send + Sync + 'static, @@ -236,7 +269,7 @@ mod tests { use tokio::net::TcpListener; use super::{ - ApiError, ApiFailure, api_failure_for, classify_api_error, map_api_error, + ApiError, ApiFailure, FABRO_VERSION, api_failure_for, classify_api_error, map_api_error, raw_response_failure_error, }; @@ -286,6 +319,48 @@ mod tests { assert_eq!(exit::exit_code_for(&err), 4); } + // A payload the server sent happily but the generated client cannot fit + // into the type it was built against, which is what a version skew looks + // like from the CLI's side. + fn invalid_payload_error(body: &str) -> progenitor_client::Error { + let source = serde_json::from_str::(body) + .expect_err("an object should not deserialize into the expected type"); + progenitor_client::Error::InvalidResponsePayload( + bytes::Bytes::copy_from_slice(body.as_bytes()), + source, + ) + } + + #[test] + fn map_api_error_explains_schema_mismatch_as_a_version_skew() { + let err = map_api_error(invalid_payload_error(r#"{"title":"some run"}"#)); + let rendered = format!("{err}"); + + assert!( + rendered.contains("did not match the schema this CLI expects"), + "{rendered}" + ); + assert!(rendered.contains("fabro version"), "{rendered}"); + assert!(rendered.contains("fabro upgrade"), "{rendered}"); + assert!(rendered.contains("--prerelease"), "{rendered}"); + assert!(rendered.contains(FABRO_VERSION), "{rendered}"); + } + + #[test] + fn schema_mismatch_truncates_the_response_body() { + let body = format!(r#"{{"title":"{}"}}"#, "x".repeat(5_000)); + let err = map_api_error(invalid_payload_error(&body)); + let rendered = format!("{err}"); + + assert!(rendered.contains("Response body started with:"), "{rendered}"); + assert!(rendered.contains("..."), "{rendered}"); + assert!( + rendered.len() < 1_000, + "message should stay readable, got {} chars:\n{rendered}", + rendered.len() + ); + } + #[test] fn map_api_error_keeps_500_as_exit_1() { let err = map_api_error(error_response(