Skip to content

Commit 5c8249d

Browse files
authored
feat(cargo-mono): improve runtime error messaging with actionable hints (#292)
## Summary - standardize cargo-mono runtime errors to a single-line `summary + Hint:` format - add stable error-kind labels and include them in stderr output (`cargo-mono error [kind]: ...`) - improve actionable runtime error messages across git/workspace/versioning/targeting/publish flows - improve publish failure details in JSON (`failed[].error`) with hint-based fallback messages - sync cargo-mono docs with the runtime error UX contract ## Testing - `cargo test -p cargo-mono` - `cargo test`
1 parent 35291ff commit 5c8249d

10 files changed

Lines changed: 272 additions & 59 deletions

File tree

crates/cargo-mono/src/commands/publish.rs

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use tracing::{info, warn};
1717
use crate::{
1818
cli::PublishArgs,
1919
commands::{print_output, targeting},
20-
errors::{CargoMonoError, Result},
20+
errors::{message_with_hint, CargoMonoError, ErrorKind, Result},
2121
types::{OutputFormat, PublishSkipReason},
2222
workspace::Workspace,
2323
CargoMonoApp,
@@ -322,11 +322,11 @@ pub fn execute(args: &PublishArgs, output: OutputFormat, app: &CargoMonoApp) ->
322322
failed.push(FailedPackage {
323323
name: package_name.clone(),
324324
attempts,
325-
error: if details.is_empty() {
326-
format!("publish failed with status {}", publish_output.status)
327-
} else {
328-
details
329-
},
325+
error: format_publish_failure(
326+
&package_name,
327+
&publish_output.status.to_string(),
328+
&details,
329+
),
330330
});
331331
published_or_skipped = true;
332332
break;
@@ -336,9 +336,9 @@ pub fn execute(args: &PublishArgs, output: OutputFormat, app: &CargoMonoApp) ->
336336

337337
if !published_or_skipped {
338338
failed.push(FailedPackage {
339-
name: package_name,
339+
name: package_name.clone(),
340340
attempts,
341-
error: "publish did not complete within retry limit".to_string(),
341+
error: format_publish_retry_limit_failure(&package_name, attempts),
342342
});
343343
}
344344
}
@@ -851,9 +851,40 @@ fn run_publish_command(package: &str, dry_run: bool, registry: Option<&str>) ->
851851
command.arg("--registry").arg(registry);
852852
}
853853

854-
command
855-
.output()
856-
.map_err(|error| CargoMonoError::cargo(format!("Failed to execute cargo publish: {error}")))
854+
command.output().map_err(|error| {
855+
CargoMonoError::with_hint(
856+
ErrorKind::Cargo,
857+
format!("Failed to start `cargo publish` for package `{package}`: {error}"),
858+
"Ensure Cargo is installed, the package exists, and registry credentials are \
859+
configured before retrying.",
860+
)
861+
})
862+
}
863+
864+
fn format_publish_failure(package: &str, status: &str, raw_details: &str) -> String {
865+
let details = compact_error_details(raw_details);
866+
let summary = if details.is_empty() {
867+
format!("`cargo publish` failed for package `{package}` with status {status}.")
868+
} else {
869+
format!("`cargo publish` failed for package `{package}`: {details}")
870+
};
871+
message_with_hint(
872+
summary,
873+
"Verify package metadata, registry access, and network connectivity, then retry.",
874+
)
875+
}
876+
877+
fn format_publish_retry_limit_failure(package: &str, attempts: usize) -> String {
878+
message_with_hint(
879+
format!(
880+
"`cargo publish` did not complete for package `{package}` within {attempts} attempts."
881+
),
882+
"Wait for index propagation or rate limits to clear, then rerun publish.",
883+
)
884+
}
885+
886+
fn compact_error_details(raw: &str) -> String {
887+
raw.split_whitespace().collect::<Vec<_>>().join(" ")
857888
}
858889

859890
fn retry_delay(attempt: usize) -> Duration {
@@ -961,6 +992,30 @@ mod tests {
961992
));
962993
}
963994

995+
#[test]
996+
fn format_publish_failure_uses_status_when_no_details_exist() {
997+
let message = format_publish_failure("alpha", "exit status: 101", "");
998+
assert!(message
999+
.contains("`cargo publish` failed for package `alpha` with status exit status: 101."));
1000+
assert!(message.contains("Hint: "));
1001+
}
1002+
1003+
#[test]
1004+
fn format_publish_failure_compacts_multiline_details() {
1005+
let message = format_publish_failure("alpha", "ignored", "error:\nnetwork timeout\n");
1006+
assert!(
1007+
message.contains("`cargo publish` failed for package `alpha`: error: network timeout")
1008+
);
1009+
assert!(message.contains("Hint: "));
1010+
}
1011+
1012+
#[test]
1013+
fn format_publish_retry_limit_failure_includes_hint() {
1014+
let message = format_publish_retry_limit_failure("alpha", 3);
1015+
assert!(message.contains("within 3 attempts."));
1016+
assert!(message.contains("Hint: "));
1017+
}
1018+
9641019
#[test]
9651020
fn sparse_index_path_matches_registry_rules() {
9661021
assert_eq!(sparse_index_path_for_crate("a"), "1/a");

crates/cargo-mono/src/commands/targeting.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::collections::BTreeSet;
22

33
use crate::{
44
cli::{ChangedArgs, TargetArgs},
5-
errors::{CargoMonoError, Result},
5+
errors::{CargoMonoError, ErrorKind, Result},
66
git,
77
types::TargetSelector,
88
workspace::Workspace,
@@ -48,10 +48,11 @@ pub fn resolve_targets(
4848
.collect::<Vec<_>>();
4949

5050
if !missing.is_empty() {
51-
return Err(CargoMonoError::invalid_input(format!(
52-
"Unknown package(s): {}",
53-
missing.join(", ")
54-
)));
51+
return Err(CargoMonoError::with_hint(
52+
ErrorKind::InvalidInput,
53+
format!("Unknown package selector(s): {}", missing.join(", ")),
54+
"Run `cargo mono list` to view valid workspace package names.",
55+
));
5556
}
5657

5758
return Ok(ResolvedTargets {

crates/cargo-mono/src/errors.rs

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@ impl ErrorKind {
2323
Self::Conflict => 5,
2424
}
2525
}
26+
27+
pub fn label(self) -> &'static str {
28+
match self {
29+
Self::Internal => "internal",
30+
Self::InvalidInput => "invalid-input",
31+
Self::Git => "git",
32+
Self::Cargo => "cargo",
33+
Self::Conflict => "conflict",
34+
}
35+
}
2636
}
2737

2838
#[derive(Debug, Error)]
@@ -40,6 +50,10 @@ impl CargoMonoError {
4050
}
4151
}
4252

53+
pub fn with_hint(kind: ErrorKind, summary: impl AsRef<str>, hint: impl AsRef<str>) -> Self {
54+
Self::new(kind, message_with_hint(summary, hint))
55+
}
56+
4357
pub fn internal(message: impl Into<String>) -> Self {
4458
Self::new(ErrorKind::Internal, message)
4559
}
@@ -67,31 +81,51 @@ impl CargoMonoError {
6781

6882
impl From<io::Error> for CargoMonoError {
6983
fn from(value: io::Error) -> Self {
70-
Self::internal(format!("I/O error: {value}"))
84+
Self::with_hint(
85+
ErrorKind::Internal,
86+
format!("I/O operation failed: {value}"),
87+
"Verify paths and permissions, then retry the command.",
88+
)
7189
}
7290
}
7391

7492
impl From<cargo_metadata::Error> for CargoMonoError {
7593
fn from(value: cargo_metadata::Error) -> Self {
76-
Self::cargo(format!("cargo metadata error: {value}"))
94+
Self::with_hint(
95+
ErrorKind::Cargo,
96+
format!("Failed to load workspace metadata via cargo: {value}"),
97+
"Run `cargo metadata` in this workspace to reproduce and inspect the root cause.",
98+
)
7799
}
78100
}
79101

80102
impl From<serde_json::Error> for CargoMonoError {
81103
fn from(value: serde_json::Error) -> Self {
82-
Self::internal(format!("JSON error: {value}"))
104+
Self::with_hint(
105+
ErrorKind::Internal,
106+
format!("Failed to parse JSON content: {value}"),
107+
"Check the JSON payload for syntax issues near the reported location.",
108+
)
83109
}
84110
}
85111

86112
impl From<semver::Error> for CargoMonoError {
87113
fn from(value: semver::Error) -> Self {
88-
Self::invalid_input(format!("Invalid semantic version: {value}"))
114+
Self::with_hint(
115+
ErrorKind::InvalidInput,
116+
format!("Invalid semantic version: {value}"),
117+
"Use a valid SemVer value such as `1.2.3` or `1.2.3-rc.1`.",
118+
)
89119
}
90120
}
91121

92122
impl From<toml_edit::TomlError> for CargoMonoError {
93123
fn from(value: toml_edit::TomlError) -> Self {
94-
Self::internal(format!("TOML error: {value}"))
124+
Self::with_hint(
125+
ErrorKind::Internal,
126+
format!("Failed to parse TOML document: {value}"),
127+
"Check TOML syntax in Cargo manifests and retry.",
128+
)
95129
}
96130
}
97131

@@ -101,6 +135,52 @@ impl From<CargoMonoError> for io::Error {
101135
}
102136
}
103137

104-
pub fn with_context<E: fmt::Display>(kind: ErrorKind, context: &str, error: E) -> CargoMonoError {
105-
CargoMonoError::new(kind, format!("{context}: {error}"))
138+
pub fn message_with_hint(summary: impl AsRef<str>, hint: impl AsRef<str>) -> String {
139+
format!("{} Hint: {}", summary.as_ref(), hint.as_ref())
140+
}
141+
142+
pub fn with_context<E: fmt::Display>(
143+
kind: ErrorKind,
144+
context: &str,
145+
error: E,
146+
hint: &str,
147+
) -> CargoMonoError {
148+
CargoMonoError::with_hint(kind, format!("{context}: {error}"), hint)
149+
}
150+
151+
#[cfg(test)]
152+
mod tests {
153+
use super::{message_with_hint, CargoMonoError, ErrorKind};
154+
155+
#[test]
156+
fn error_kind_labels_are_stable() {
157+
assert_eq!(ErrorKind::Internal.label(), "internal");
158+
assert_eq!(ErrorKind::InvalidInput.label(), "invalid-input");
159+
assert_eq!(ErrorKind::Git.label(), "git");
160+
assert_eq!(ErrorKind::Cargo.label(), "cargo");
161+
assert_eq!(ErrorKind::Conflict.label(), "conflict");
162+
}
163+
164+
#[test]
165+
fn message_with_hint_uses_single_line_contract() {
166+
let message = message_with_hint("Unable to read manifest.", "Check file permissions.");
167+
assert_eq!(
168+
message,
169+
"Unable to read manifest. Hint: Check file permissions."
170+
);
171+
}
172+
173+
#[test]
174+
fn with_hint_formats_error_message() {
175+
let error = CargoMonoError::with_hint(
176+
ErrorKind::InvalidInput,
177+
"Invalid package selector.",
178+
"Run `cargo mono list`.",
179+
);
180+
assert_eq!(error.kind, ErrorKind::InvalidInput);
181+
assert_eq!(
182+
error.message,
183+
"Invalid package selector. Hint: Run `cargo mono list`."
184+
);
185+
}
106186
}

crates/cargo-mono/src/git.rs

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ pub fn merge_base(base_ref: &str) -> Result<String> {
2323
ErrorKind::Git,
2424
&format!("Failed to resolve merge-base for base ref `{base_ref}`"),
2525
error,
26+
"Ensure the base ref exists locally (for example, run `git fetch`) and retry with \
27+
`--base <ref>`.",
2628
)
2729
})
2830
}
@@ -59,8 +61,10 @@ pub fn ensure_clean_working_tree(allow_dirty: bool) -> Result<()> {
5961
return Ok(());
6062
}
6163

62-
Err(CargoMonoError::conflict(
63-
"Working tree is dirty; re-run with --allow-dirty to bypass this check",
64+
Err(CargoMonoError::with_hint(
65+
ErrorKind::Conflict,
66+
"Working tree is dirty and cannot pass preflight checks.",
67+
"Commit or stash local changes, or rerun with `--allow-dirty` when this is intentional.",
6468
))
6569
}
6670

@@ -112,10 +116,14 @@ fn parse_paths(output: &str) -> BTreeSet<PathBuf> {
112116
}
113117

114118
fn run_git(args: &[&str]) -> Result<Output> {
115-
let output = Command::new("git")
116-
.args(args)
117-
.output()
118-
.map_err(|error| with_context(ErrorKind::Git, "Failed to execute git", error))?;
119+
let output = Command::new("git").args(args).output().map_err(|error| {
120+
with_context(
121+
ErrorKind::Git,
122+
"Failed to start `git`",
123+
error,
124+
"Ensure `git` is installed and available in PATH.",
125+
)
126+
})?;
119127

120128
ensure_success(&output, args.join(" "))?;
121129
Ok(output)
@@ -125,7 +133,14 @@ fn run_git_os(args: Vec<OsString>) -> Result<Output> {
125133
let output = Command::new("git")
126134
.args(args.iter().map(OsString::as_os_str))
127135
.output()
128-
.map_err(|error| with_context(ErrorKind::Git, "Failed to execute git", error))?;
136+
.map_err(|error| {
137+
with_context(
138+
ErrorKind::Git,
139+
"Failed to start `git`",
140+
error,
141+
"Ensure `git` is installed and available in PATH.",
142+
)
143+
})?;
129144

130145
let command = args
131146
.iter()
@@ -147,11 +162,18 @@ fn ensure_success(output: &Output, command: String) -> Result<()> {
147162
}
148163

149164
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
150-
let message = if stderr.is_empty() {
151-
format!("git {command} failed with status {}", output.status)
165+
let summary = if stderr.is_empty() {
166+
format!(
167+
"Git command `git {command}` failed with status {}.",
168+
output.status
169+
)
152170
} else {
153-
format!("git {command} failed: {stderr}")
171+
format!("Git command `git {command}` failed: {stderr}")
154172
};
155173

156-
Err(CargoMonoError::git(message))
174+
Err(CargoMonoError::with_hint(
175+
ErrorKind::Git,
176+
summary,
177+
format!("Run `git {command}` directly to inspect and resolve the underlying problem."),
178+
))
157179
}

crates/cargo-mono/src/main.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ fn main() {
1212
match run() {
1313
Ok(code) => std::process::exit(code),
1414
Err(error) => {
15-
eprintln!("cargo-mono error: {}", error.message);
15+
eprintln!(
16+
"cargo-mono error [{}]: {}",
17+
error.kind.label(),
18+
error.message
19+
);
1620
std::process::exit(error.exit_code());
1721
}
1822
}

crates/cargo-mono/src/versioning.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use semver::{Prerelease, Version};
88
use toml_edit::{value, DocumentMut, Item, Value};
99

1010
use crate::{
11-
errors::{CargoMonoError, Result},
11+
errors::{CargoMonoError, ErrorKind, Result},
1212
types::BumpLevel,
1313
workspace::Workspace,
1414
};
@@ -46,7 +46,11 @@ pub fn bump_version(current: &Version, level: BumpLevel, preid: Option<&str>) ->
4646
}
4747
BumpLevel::Prerelease => {
4848
let preid = preid.ok_or_else(|| {
49-
CargoMonoError::invalid_input("--preid is required when --level prerelease")
49+
CargoMonoError::with_hint(
50+
ErrorKind::InvalidInput,
51+
"`--preid` is required when `--level prerelease` is used.",
52+
"Provide a prerelease identifier, for example `--preid rc`.",
53+
)
5054
})?;
5155

5256
let next_pre = next_prerelease(current, preid)?;

0 commit comments

Comments
 (0)