mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMb7GgUkEkk8CWc6MPpy2w
This commit is contained in:
parent
7200d437e9
commit
c67ef194a9
1 changed files with 76 additions and 1 deletions
|
|
@ -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<E>(err: progenitor_client::Error<E>) -> 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<serde_json::Value> {
|
||||
let source = serde_json::from_str::<u32>(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(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue