fix: Docker not found error, better UX #56
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the user experience when Docker CLI is not installed by adding a friendly error message instead of the default "No such file or directory" error. The change is localized to the init command's cleanup function.
Changes:
- Adds
docker_not_found_error()helper function that returns a user-friendly error message - Wraps two Docker command invocations (
docker imagesanddocker rmi) with error handling that detectsErrorKind::NotFoundand returns the friendly message
src/commands/init/mod.rs
Outdated
| "Docker CLI not found. Install Docker Desktop and ensure the 'docker' command is on PATH, then re-run 'foc-devnet init'." | ||
| .to_string() | ||
| .into() |
There was a problem hiding this comment.
The error message string should be defined as a constant rather than being hardcoded. According to the project's coding guidelines, magic strings should be extracted to constants, typically in src/constants.rs. This improves maintainability and allows the message to be reused if needed elsewhere in the codebase.
Consider defining a constant like:
pub const DOCKER_NOT_FOUND_ERROR: &str = "Docker CLI not found. Install Docker Desktop and ensure the 'docker' command is on PATH, then re-run 'foc-devnet init'.";| .output() | ||
| .map_err(|err| match err.kind() { | ||
| ErrorKind::NotFound => docker_not_found_error(), | ||
| _ => err.into(), | ||
| })?; |
There was a problem hiding this comment.
This error handling for Docker CLI not found is only applied in the init command's cleanup function, but there are 20+ other locations throughout the codebase that use Command::new("docker") without this error handling. This creates an inconsistent user experience where users might get a helpful error message here but a generic "No such file or directory" error elsewhere.
Consider centralizing this error handling in the docker_command function in src/docker/core.rs (line 51-53), which already serves as the centralized wrapper for Docker commands. This would provide consistent error messages across the entire codebase without requiring changes to every call site.
For example, the run_command function in src/docker/core.rs could be enhanced to detect ErrorKind::NotFound and provide context about which program was not found.
src/commands/init/mod.rs
Outdated
|
|
||
| /// Create a friendly error for missing Docker CLI. | ||
| fn docker_not_found_error() -> Box<dyn std::error::Error> { | ||
| "Docker CLI not found. Install Docker Desktop and ensure the 'docker' command is on PATH, then re-run 'foc-devnet init'." |
There was a problem hiding this comment.
maybe just "Install Docker" since we don't actually need Desktop?
rvagg
left a comment
rvagg
left a comment
There was a problem hiding this comment.
TIL about std::io::ErrorKind::NotFound
github-project-automation
bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
No description provided.