Skip to content

Commit

Permalink
Improve io error message context
Browse files Browse the repository at this point in the history
- include the failed recipe, shell and working directory
- include the original io error message from Command.status()
- include the io error message from read_dir() check if it differs from
  the original

On Unix the two error messages don't differ in the cases that we test,
but we don't want to assume that is always the case.

Additionally test error messages when both workdir and shell are
unusable.
  • Loading branch information
artm committed Sep 1, 2024
1 parent 9307ddc commit bb95c38
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 32 deletions.
26 changes: 12 additions & 14 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ pub(crate) enum Error<'src> {
Io {
recipe: &'src str,
io_error: io::Error,
shell: String,
working_directory_io_error: Option<io::Error>,
working_directory: Option<PathBuf>,
},
Load {
path: PathBuf,
Expand Down Expand Up @@ -186,11 +189,6 @@ pub(crate) enum Error<'src> {
justfile: PathBuf,
io_error: io::Error,
},
WorkingDirectoryIo {
recipe: &'src str,
working_directory: PathBuf,
io_error: io::Error,
},
}

impl<'src> Error<'src> {
Expand Down Expand Up @@ -401,12 +399,15 @@ impl<'src> ColorDisplay for Error<'src> {
write!(f, "Internal runtime error, this may indicate a bug in just: {message} \
consider filing an issue: https://github.com/casey/just/issues/new")?;
}
Io { recipe, io_error } => {
match io_error.kind() {
io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"),
io::ErrorKind::PermissionDenied => write!(f, "Recipe `{recipe}` could not be run because just could not run the shell: {io_error}"),
_ => write!(f, "Recipe `{recipe}` could not be run because of an IO error while launching the shell: {io_error}"),
}?;
Io { recipe, io_error, shell, working_directory_io_error, working_directory } => {
write!(f, "Failed to run recipe `{recipe}`:\n Failed to run shell `{shell}`:\n {io_error}", )?;
if let Some(working_directory_io_error) = working_directory_io_error {
let working_directory = working_directory.as_ref().expect("is Some when error is Some").display();
write!(f, "\n Failed to set working directory to `{working_directory}`")?;
if working_directory_io_error.to_string() != io_error.to_string() {
write!(f, ":\n {working_directory_io_error}")?;
}
}
}
Load { io_error, path } => {
write!(f, "Failed to read justfile at `{}`: {io_error}", path.display())?;
Expand Down Expand Up @@ -475,9 +476,6 @@ impl<'src> ColorDisplay for Error<'src> {
UnstableFeature { unstable_feature } => {
write!(f, "{unstable_feature} Invoke `just` with `--unstable`, set the `JUST_UNSTABLE` environment variable, or add `set unstable` to your `justfile` to enable unstable features.")?;
}
WorkingDirectoryIo { recipe, working_directory, io_error } => {
write!(f, "Recipe `{recipe}` could not be run because just could not set working directory to `{}`: {io_error}", working_directory.display())?;
},
WriteJustfile { justfile, io_error } => {
let justfile = justfile.display();
write!(f, "Failed to write justfile to `{justfile}`: {io_error}")?;
Expand Down
23 changes: 14 additions & 9 deletions src/recipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,18 +304,23 @@ impl<'src, D> Recipe<'src, D> {
}
}
Err(io_error) => {
if let Some(working_directory) = self.working_directory(context) {
if let Err(io_error) = fs::read_dir(&working_directory) {
return Err(Error::WorkingDirectoryIo {
recipe: self.name(),
working_directory,
io_error,
});
}
}
let working_directory = self.working_directory(context);
let working_directory_io_error = match &working_directory {
Some(working_directory) => match fs::read_dir(&working_directory) {
Ok(_) => None,
Err(io_error) => Some(io_error),
},
// no working directory = no attempt to change current working directory = no error
None => None,
};
let (shell, _) = context.module.settings.shell(config);

return Err(Error::Io {
recipe: self.name(),
io_error,
shell: shell.into(),
working_directory_io_error,
working_directory,
});
}
};
Expand Down
2 changes: 1 addition & 1 deletion tests/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ fn recipe_shell_not_found_error_message() {
.shell(false)
.args(["--shell", "NOT_A_REAL_SHELL"])
.stderr_regex(
"error: Recipe `foo` could not be run because just could not find the shell: .*\n",
".*Failed to run recipe `foo`:\n Failed to run shell `NOT_A_REAL_SHELL`:\n .*",
)
.status(1)
.run();
Expand Down
70 changes: 62 additions & 8 deletions tests/working_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,9 +343,7 @@ fn missing_working_directory_produces_clear_message() {
",
)
.status(1)
.stderr_regex(
".*Recipe `default` could not be run because just could not set working directory to `.*/missing`.*",
)
.stderr_regex(".*Failed to run recipe `default`:\n Failed to run shell `bash`:\n .*\n Failed to set working directory to `.*/missing`.*")
.run();
}

Expand All @@ -366,9 +364,7 @@ fn unusable_working_directory_produces_clear_message() {
})
.chmod("unusable", Permissions::from_mode(0o000))
.status(1)
.stderr_regex(
".*Recipe `default` could not be run because just could not set working directory to `.*/unusable`:.*",
)
.stderr_regex(".*Failed to run recipe `default`:\n Failed to run shell `bash`:\n .*\n Failed to set working directory to `.*/unusable`.*")
.run();
}

Expand All @@ -386,8 +382,66 @@ fn working_directory_is_not_a_directory_produces_clear_message() {
unusable: "is not a directory"
})
.status(1)
.stderr_regex(
".*Recipe `default` could not be run because just could not set working directory to `.*/unusable`:.*",
.stderr_regex(".*Failed to run recipe `default`:\n Failed to run shell `bash`:\n .*\n Failed to set working directory to `.*/unusable`.*")
.run();
}

#[test]
fn missing_working_directory_and_missing_shell_produces_clear_message() {
Test::new()
.justfile(
"
set working-directory := 'missing'
default:
pwd
",
)
.shell(false)
.args(["--shell", "NOT_A_REAL_SHELL"])
.status(1)
.stderr_regex(".*Failed to run recipe `default`:\n Failed to run shell `NOT_A_REAL_SHELL`:\n .*\n Failed to set working directory to `.*/missing`.*")
.run();
}

#[test]
#[cfg(unix)]
fn unusable_working_directory_and_missing_shell_produces_clear_message() {
use {fs::Permissions, std::os::unix::fs::PermissionsExt};
Test::new()
.justfile(
"
set working-directory := 'unusable'
default:
pwd
",
)
.tree(tree! {
unusable: {}
})
.shell(false)
.args(["--shell", "NOT_A_REAL_SHELL"])
.chmod("unusable", Permissions::from_mode(0o000))
.status(1)
.stderr_regex(".*Failed to run recipe `default`:\n Failed to run shell `NOT_A_REAL_SHELL`:\n .*\n Failed to set working directory to `.*/unusable`.*")
.run();
}

#[test]
fn working_directory_is_not_a_directory_and_missing_shell_produces_clear_message() {
Test::new()
.justfile(
"
set working-directory := 'unusable'
default:
pwd
",
)
.tree(tree! {
unusable: "is not a directory"
})
.shell(false)
.args(["--shell", "NOT_A_REAL_SHELL"])
.status(1)
.stderr_regex(".*Failed to run recipe `default`:\n Failed to run shell `NOT_A_REAL_SHELL`:\n .*\n Failed to set working directory to `.*/unusable`.*")
.run();
}

0 comments on commit bb95c38

Please sign in to comment.