Follow-up from PR #222.
Background
PR #222 added gh-style exit codes. CloudError now carries a kind (Auth / Generic) so 401/403 and missing-creds map to exit 4. But cloud command handlers (cloud/commands.rs, cloud/postgres.rs) and run_postgres return Box<dyn std::error::Error>, which type-erases the CloudError. To recover kind at the top level we currently downcast:
fn boxed_cloud_error_to_top_level(e: Box<dyn std::error::Error>) -> Error {
match e.downcast::<cloud::CloudError>() {
Ok(ce) => cloud_error_to_top_level(*ce), // kind recovered -> exit 4 possible
Err(other) => Error::Cloud(other.to_string()), // not a CloudError -> exit 1
}
}
Problem
The downcast only succeeds if the boxed concrete type is exactly CloudError. If any handler stringifies a CloudError before boxing — e.g. client.foo().await.map_err(|e| format!("...: {e}"))? instead of plain ? — the downcast misses and exit 4 silently degrades to exit 1. It still compiles, the message still prints, the process still exits non-zero; only the machine-readable auth signal is lost. Unit tests pin the router contract, but nothing structurally prevents a future handler from breaking it.
Box<dyn Error> was chosen for ergonomic ? across heterogeneous error sources (CloudError, serde_json, uuid, chrono, io, plain validation strings) — that convenience is exactly what erases the type we now need to inspect.
Proposed improvement
Have cloud handlers return a concrete, matchable error type instead of Box<dyn Error> — e.g. the top-level Error (or a dedicated cloud error enum) with #[from] impls for the parse/io/cloud sources. Then kind is carried as Error::AuthRequired vs Error::Cloud from the point of failure, main matches on it directly, and boxed_cloud_error_to_top_level + the downcast disappear entirely.
Scope / cost
Touches the signature and error construction of every cloud handler in cloud/commands.rs and cloud/postgres.rs, plus run_postgres. Larger than #222, hence split out. No user-facing behavior change — purely making the exit-code routing structurally correct rather than downcast-dependent.
Follow-up from PR #222.
Background
PR #222 added
gh-style exit codes.CloudErrornow carries akind(Auth/Generic) so 401/403 and missing-creds map to exit4. But cloud command handlers (cloud/commands.rs,cloud/postgres.rs) andrun_postgresreturnBox<dyn std::error::Error>, which type-erases theCloudError. To recoverkindat the top level we currently downcast:Problem
The downcast only succeeds if the boxed concrete type is exactly
CloudError. If any handler stringifies aCloudErrorbefore boxing — e.g.client.foo().await.map_err(|e| format!("...: {e}"))?instead of plain?— the downcast misses and exit4silently degrades to exit1. It still compiles, the message still prints, the process still exits non-zero; only the machine-readable auth signal is lost. Unit tests pin the router contract, but nothing structurally prevents a future handler from breaking it.Box<dyn Error>was chosen for ergonomic?across heterogeneous error sources (CloudError, serde_json, uuid, chrono, io, plain validation strings) — that convenience is exactly what erases the type we now need to inspect.Proposed improvement
Have cloud handlers return a concrete, matchable error type instead of
Box<dyn Error>— e.g. the top-levelError(or a dedicated cloud error enum) with#[from]impls for the parse/io/cloud sources. Thenkindis carried asError::AuthRequiredvsError::Cloudfrom the point of failure,mainmatches on it directly, andboxed_cloud_error_to_top_level+ the downcast disappear entirely.Scope / cost
Touches the signature and error construction of every cloud handler in
cloud/commands.rsandcloud/postgres.rs, plusrun_postgres. Larger than #222, hence split out. No user-facing behavior change — purely making the exit-code routing structurally correct rather than downcast-dependent.