fix(cli): suppress error output after interactive cancel#9294
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces logic to detect and handle user-initiated cancellations by checking the error chain for a specific message. While this allows for cleaner exits, the reviewer noted that returning a success status (exit code 0) might negatively impact shell scripts that rely on non-zero exit codes for failures. Additionally, the reviewer suggested improving the robustness of the error detection by using case-insensitive matching or a dedicated error type instead of a fragile exact string comparison.
| return Ok(()); | ||
| } | ||
| if is_user_cancelled(&err) { | ||
| return Ok(()); |
There was a problem hiding this comment.
Returning Ok(()) here results in an exit code of 0. While this suppresses the error output and allows the cleanup logic (like mpr.stop()) to run, it also signals to the calling shell or script that the command succeeded. Typically, a user cancellation should result in a non-zero exit code (e.g., 1) to ensure that dependent commands (like those following &&) do not execute. Consider if there is a way to exit with a non-zero status while still ensuring terminal cleanup.
| } | ||
|
|
||
| fn is_user_cancelled(err: &Report) -> bool { | ||
| err.chain().any(|err| err.to_string() == "user cancelled") |
There was a problem hiding this comment.
Matching against the exact string "user cancelled" is fragile. If the error message is changed in the future (e.g., to "User cancelled" or "user canceled"), this check will silently stop working. It would be more robust to use a specific error type that can be downcast, or at least perform a case-insensitive comparison. Also, consider renaming the closure argument to avoid shadowing the outer err variable.
| err.chain().any(|err| err.to_string() == "user cancelled") | |
| err.chain().any(|e| e.to_string().to_lowercase() == "user cancelled") |
Greptile SummaryThis PR suppresses noisy error output when a user dismisses an interactive prompt by detecting Confidence Score: 5/5Safe to merge — narrow, well-tested change that correctly exits 130 on interactive cancellation. All remaining findings are P2. The implementation is typed (ErrorKind::Interrupted) rather than string-based, exit code is correct per POSIX, and there is a unit test. Prior review concerns have been resolved. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Cli::run returns Err] --> B[handle_err]
B --> C{BrokenPipe?}
C -- yes --> D[return Ok / exit 0]
C -- no --> E{ErrorKind::Interrupted\nin chain?}
E -- yes --> F[stop_multi_progress\nexit 130]
E -- no --> G{MiseDiagnostic?}
G -- yes --> H[eprintln diagnostic\nexit 1]
G -- no --> I{MISE_FRIENDLY_ERROR?}
I -- yes --> J[display_friendly_err\nexit 1]
I -- no --> K[propagate Err with\nasync backtrace]
Reviews (2): Last reviewed commit: "fix(cli): use interrupted io error for i..." | Re-trigger Greptile |
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.18 x -- echo |
26.5 ± 0.4 | 24.9 | 27.8 | 1.00 |
mise x -- echo |
27.1 ± 0.5 | 25.8 | 34.1 | 1.02 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.18 env |
25.1 ± 1.3 | 22.8 | 39.5 | 1.00 |
mise env |
26.5 ± 0.4 | 25.1 | 28.2 | 1.06 ± 0.06 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.18 hook-env |
26.8 ± 1.2 | 24.5 | 42.5 | 1.00 |
mise hook-env |
26.8 ± 1.1 | 24.3 | 38.0 | 1.00 ± 0.06 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.4.18 ls |
23.8 ± 0.9 | 22.5 | 36.8 | 1.00 |
mise ls |
24.5 ± 0.5 | 23.0 | 26.0 | 1.03 ± 0.04 |
xtasks/test/perf
| Command | mise-2026.4.18 | mise | Variance |
|---|---|---|---|
| install (cached) | 153ms | 159ms | -3% |
| ls (cached) | 82ms | 84ms | -2% |
| bin-paths (cached) | 87ms | 88ms | -1% |
| task-ls (cached) | 805ms | 817ms | -1% |
The `cargo clippy --all-features --all-targets -- -D warnings` CI job was
failing on `src/main.rs:255-258`:
std::io::Error::new(std::io::ErrorKind::Other, "user cancelled")
Clippy 1.94+ flags this as `clippy::io_other_error`, preferring
`std::io::Error::other("user cancelled")`. The local clippy invocation
during PR jdx#9307's review phase used `--bin mise`, which does not compile
the test target and therefore did not surface this. The failure is not
introduced by this branch (the offending code came in via jdx#9294), but
the branch can't merge without fixing it.
The sibling `Error::new(ErrorKind::Interrupted, ...)` on the line above
is intentionally left alone — clippy does not flag it (and should not;
`Error::other` is specific to `ErrorKind::Other`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
user cancelledinteractive picker error as a graceful exit in the top-level error handlerNote
Low Risk
Narrow, top-level error-handling change gated on
io::ErrorKind::Interrupted; main risk is altering exit behavior/output for some cancellation paths.Overview
Treats
std::io::ErrorKind::Interruptedanywhere in the error chain as a graceful user cancel in the top-level CLI error handler, stopping any activeMultiProgressReportand exiting with code130instead of printing friendly/diagnostic output.Adds small helpers (
is_interrupted_io_error,stop_multi_progress) plus a focused unit test to ensure onlyInterruptedis handled this way (not other IO errors).Reviewed by Cursor Bugbot for commit 6fae676. Bugbot is set up for automated code reviews on this repo. Configure here.