diff --git a/nvx-rs/src/host/cli/artifacts.rs b/nvx-rs/src/host/cli/artifacts.rs index e0f19c86c..d8a7b35e7 100644 --- a/nvx-rs/src/host/cli/artifacts.rs +++ b/nvx-rs/src/host/cli/artifacts.rs @@ -49,8 +49,7 @@ pub(super) fn run(root: Option<&str>) -> Result { paths.openvmm = located.openvmm; let value = serde_json::to_value(&paths) .map_err(|error| Exit::error(format!("cannot encode the artifact paths: {error}")))?; - output::print_stdout(&dumps::dumps(&json_value(&value))) - .map_err(|error| Exit::error(output::stream_error(&error))) + output::print_stdout(&dumps::dumps(&json_value(&value))).map_err(Exit::from_os_error) } /// The artifacts that a command runs. diff --git a/nvx-rs/src/host/cli/egress.rs b/nvx-rs/src/host/cli/egress.rs index a73c7a723..5c211fac0 100644 --- a/nvx-rs/src/host/cli/egress.rs +++ b/nvx-rs/src/host/cli/egress.rs @@ -80,6 +80,5 @@ pub(super) fn run(path: &str) -> Result { let policy = compile_policy_file(Path::new(path))?; let rules = |rules: Vec| Value::List(rules.into_iter().map(Value::Str).collect()); let value = dumps::dict([("allow", rules(policy.allow)), ("deny", rules(policy.deny))]); - output::print_stdout(&dumps::dumps(&value)) - .map_err(|error| Exit::error(output::stream_error(&error))) + output::print_stdout(&dumps::dumps(&value)).map_err(Exit::from_os_error) } diff --git a/nvx-rs/src/host/cli/mod.rs b/nvx-rs/src/host/cli/mod.rs index 26de4d070..b6915c717 100644 --- a/nvx-rs/src/host/cli/mod.rs +++ b/nvx-rs/src/host/cli/mod.rs @@ -36,6 +36,7 @@ use std::ffi::OsString; use self::args::{Parser, Receiver, Spec, Stop, Syntax}; use crate::host::platform; use crate::host::python::builtins::repr; +use crate::host::python::fs::os_error_message; /// The name of the program in usage messages unless `--prog` names another. const PROGRAM: &str = "nvx-rs"; @@ -124,6 +125,13 @@ impl Exit { } } + /// Returns the error of an `OSError` that names no file, which prints `str(error)` as + /// [`os_error_message`] formats it, such as `[Errno 32] Broken pipe` for a write to a + /// standard stream whose reader has gone. + pub(crate) fn from_os_error(error: std::io::Error) -> Self { + Self::Error(os_error_message(&error, None)) + } + /// Returns the message that the end prints after `error: `, if it is an error. pub(crate) fn message(&self) -> Option<&str> { match self { @@ -152,7 +160,7 @@ impl Exit { Ok(()) => 0, // Ctrl-C ends a help that waits for its reader, as `KeyboardInterrupt` ends it. Err(_) if platform::interrupted() => Self::Interrupted.report(), - Err(error) => Self::Error(output::stream_error(&error)).report(), + Err(error) => Self::from_os_error(error).report(), }, Self::Interrupted => { output::print_stderr("Interrupted"); @@ -291,3 +299,27 @@ fn run(arguments: &[OsString]) -> Result { Command::Sandbox(options) => sandbox::run(root, *options), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn os_errors_without_a_file_read_as_python_formats_them() { + let error = std::fs::metadata("definitely/missing/path").unwrap_err(); + let expected = os_error_message(&error, None); + let exit = Exit::from_os_error(error); + let message = exit.message().unwrap(); + assert_eq!(message, expected); + let label = if cfg!(windows) { + "[WinError 3] " + } else { + "[Errno 2] " + }; + assert!(message.starts_with(label), "{message}"); + assert!(!message.contains("os error"), "{message}"); + assert!(!message.contains("missing"), "{message}"); + let error = std::io::Error::other("closed"); + assert_eq!(Exit::from_os_error(error).message(), Some("closed")); + } +} diff --git a/nvx-rs/src/host/cli/output.rs b/nvx-rs/src/host/cli/output.rs index 4756603de..1b0a53afd 100644 --- a/nvx-rs/src/host/cli/output.rs +++ b/nvx-rs/src/host/cli/output.rs @@ -15,7 +15,6 @@ use std::io::{self, Write}; use std::os::fd::AsFd; use crate::host::platform; -use crate::host::python::fs::os_error_message; /// The line separator of this host, `os.linesep`. pub(crate) const LINE_SEPARATOR: &str = if cfg!(windows) { "\r\n" } else { "\n" }; @@ -70,12 +69,6 @@ fn write_all(stream: &mut impl Write, data: &[u8]) -> io::Result<()> { stream.flush() } -/// Returns the message of the `OSError` that writing to a standard stream raises, such as -/// `[Errno 32] Broken pipe`. -pub(crate) fn stream_error(error: &io::Error) -> String { - os_error_message(error, None) -} - #[cfg(test)] mod tests { use super::*; diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle.rs b/nvx-rs/src/host/cli/sandbox/lifecycle.rs index fe4ef1caa..9dd010828 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle.rs @@ -184,8 +184,7 @@ fn make_private(path: &PurePath) -> Result { /// Resolves the state directory at `path`, and creates it with `create`, as /// `_prepare_state_directory` does. It must be a directory and not a symbolic link. pub(crate) fn prepare_state_directory(path: &str, create: bool) -> Result { - let resolved = fs::resolve(&PurePath::native(path)) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + let resolved = fs::resolve(&PurePath::native(path)).map_err(Exit::from_os_error)?; if create { make_directories(&resolved, 0o700)?; make_private(&resolved)?; @@ -205,9 +204,7 @@ fn outcome_destination(path: &str) -> Result { let candidate = if path.is_absolute() { path } else { - fs::cwd() - .map_err(|error| Exit::error(os_error_message(&error, None)))? - .join(path.as_str()) + fs::cwd().map_err(Exit::from_os_error)?.join(path.as_str()) }; let parent = candidate .parents() @@ -222,7 +219,7 @@ fn outcome_destination(path: &str) -> Result { return Err(Exit::error("outcome report path has no filename")); } let resolved = fs::resolve(&parent) - .map_err(|error| Exit::error(os_error_message(&error, None)))? + .map_err(Exit::from_os_error)? .join(candidate.name()); if fs::lexists(resolved.as_path()) { return Err(Exit::error(format!( @@ -261,7 +258,7 @@ fn write_new_json(path: &str, value: &Value) -> Result { let text = format!("{}\n", dumps::dumps(value)); file.write_all(text.as_bytes()) .and_then(|()| file.sync_all()) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + .map_err(Exit::from_os_error)?; match std::fs::hard_link(temporary.as_path(), resolved.as_path()) { Ok(()) => Ok(()), Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => Err(Exit::error( @@ -591,8 +588,7 @@ fn launch_and_connect( launched: &mut Option, ) -> Result { let arguments: Vec = command[1..].iter().map(Into::into).collect(); - let directory = - std::env::current_dir().map_err(|error| Exit::error(os_error_message(&error, None)))?; + let directory = std::env::current_dir().map_err(Exit::from_os_error)?; let child = process::spawn( std::path::Path::new(&command[0]), false, @@ -611,7 +607,7 @@ fn launch_and_connect( let Some(start_time) = start_time else { let exited = child .wait_timeout(Duration::ZERO) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + .map_err(Exit::from_os_error)?; return Err(Exit::error(if exited.is_some() { "OpenVMM exited during startup" } else { @@ -726,21 +722,17 @@ fn end_failed_start( }; let status = launched .wait_timeout(Duration::ZERO) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + .map_err(Exit::from_os_error)?; if status.is_none() { - launched - .terminate() - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + launched.terminate().map_err(Exit::from_os_error)?; let exited = launched .wait_timeout(Duration::from_secs(5)) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + .map_err(Exit::from_os_error)?; if exited.is_none() { - launched - .kill() - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + launched.kill().map_err(Exit::from_os_error)?; let killed = launched .wait_timeout(Duration::from_secs(5)) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + .map_err(Exit::from_os_error)?; if killed.is_none() { let arguments: Vec = command.iter().map(|argument| repr(argument)).collect(); @@ -765,7 +757,7 @@ fn wait_interruptibly(launched: &mut Launched, deadline: Deadline) -> Result { crate::host::cli::check_interrupt()?; if launched .wait_timeout(slice) - .map_err(|error| Exit::error(os_error_message(&error, None)))? + .map_err(Exit::from_os_error)? .is_some() || deadline.passed() { @@ -931,8 +923,7 @@ fn hold_directory(path: &PurePath) -> Result<((u64, u128), Option)> { .custom_flags((nix::fcntl::OFlag::O_DIRECTORY | nix::fcntl::OFlag::O_NOFOLLOW).bits()) .open(path.as_path()) .map_err(|error| os_error(&error, path))?; - let status = - fs::fstat(&file).map_err(|error| Exit::error(os_error_message(&error, None)))?; + let status = fs::fstat(&file).map_err(Exit::from_os_error)?; Ok((status.identity, Some(file))) } #[cfg(not(unix))] diff --git a/nvx-rs/src/host/cli/sandbox/lock.rs b/nvx-rs/src/host/cli/sandbox/lock.rs index de5becec2..cd71874a4 100644 --- a/nvx-rs/src/host/cli/sandbox/lock.rs +++ b/nvx-rs/src/host/cli/sandbox/lock.rs @@ -133,8 +133,7 @@ fn open_lock_file(path: &PurePath) -> std::result::Result { Err(error) if is_loop(&error) => return Err(OpenFailure::Exit(not_standalone(path))), Err(error) => return Err(OpenFailure::Os(error)), }; - let status = fs::fstat(&file) - .map_err(|error| OpenFailure::Exit(Exit::error(os_error_message(&error, None))))?; + let status = fs::fstat(&file).map_err(|error| OpenFailure::Exit(Exit::from_os_error(error)))?; if !status.regular || status.links != 1 || status.reparse_point { return Err(OpenFailure::Exit(not_standalone(path))); } @@ -150,9 +149,7 @@ fn try_lock_file(file: &File) -> Result { match file.try_lock() { Ok(()) => Ok(true), Err(std::fs::TryLockError::WouldBlock) => Ok(false), - Err(std::fs::TryLockError::Error(error)) => { - Err(Exit::error(os_error_message(&error, None))) - } + Err(std::fs::TryLockError::Error(error)) => Err(Exit::from_os_error(error)), } } @@ -354,8 +351,8 @@ impl LifecycleLock { file: Some(file), }); } - let tombstone = read_lock_file(&file, LOCK_TOMBSTONE.len()) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + let tombstone = + read_lock_file(&file, LOCK_TOMBSTONE.len()).map_err(Exit::from_os_error)?; // Deprovision leaves the mark only if lingering lock files, such as the one that this // handle may keep, kept it from removing the directory, which the path then still // names. @@ -383,7 +380,7 @@ impl LifecycleLock { pub(crate) fn remove(&mut self) -> Result { let file = self.file.as_ref().expect("the lock is held"); let removed = write_lock_file(file, LOCK_TOMBSTONE) - .map_err(|error| Exit::error(os_error_message(&error, None))) + .map_err(Exit::from_os_error) .and_then(|()| { std::fs::remove_file(self.path.as_path()).map_err(|error| { Exit::error(os_error_message(&error, Some(self.path.as_str()))) diff --git a/nvx-rs/src/host/cli/sandbox/mod.rs b/nvx-rs/src/host/cli/sandbox/mod.rs index f2ed06754..4f413efad 100644 --- a/nvx-rs/src/host/cli/sandbox/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/mod.rs @@ -667,7 +667,7 @@ pub(super) fn run(root: Option<&str>, options: Options) -> Result { )?; output::write_stdout(&result.stdout) .and_then(|()| output::write_stderr(&result.stderr)) - .map_err(|error| Exit::error(output::stream_error(&error)))?; + .map_err(Exit::from_os_error)?; if let Some(report) = &options.outcome_report { lifecycle::write_exec_outcome(report, &result)?; } @@ -713,7 +713,7 @@ pub(super) fn run(root: Option<&str>, options: Options) -> Result { command.extend(["--microvm-report".to_owned(), path_text(report)]); } output::print_stdout(&format!(">> {}", format_command(&command))) - .map_err(|error| Exit::error(output::stream_error(&error)))?; + .map_err(Exit::from_os_error)?; if options.dry_run { return Ok(()); } @@ -770,9 +770,7 @@ fn end_interrupted(child: &mut Child) -> Result { /// Returns the status of `child` if it has exited: `Popen.returncode`, which on Unix is the /// negated signal of a process that a signal ended. fn poll(child: &mut Child) -> Result> { - let status = child - .try_wait() - .map_err(|error| Exit::error(crate::host::python::fs::os_error_message(&error, None)))?; + let status = child.try_wait().map_err(Exit::from_os_error)?; Ok(status.map(|status| { #[cfg(unix)] { diff --git a/nvx-rs/src/host/cli/sandbox/state.rs b/nvx-rs/src/host/cli/sandbox/state.rs index f9ec310b7..d1ea0cb66 100644 --- a/nvx-rs/src/host/cli/sandbox/state.rs +++ b/nvx-rs/src/host/cli/sandbox/state.rs @@ -233,8 +233,7 @@ fn lists_mounts(format: u64) -> bool { /// Returns `path.resolve()` as text. fn resolved(path: &std::path::Path) -> Result { let text = crate::host::cli::sandbox::path_string(path)?; - let resolved = fs::resolve(&PurePath::native(&text)) - .map_err(|error| Exit::error(os_error_message(&error, None)))?; + let resolved = fs::resolve(&PurePath::native(&text)).map_err(Exit::from_os_error)?; Ok(resolved.as_str().to_owned()) } diff --git a/nvx-rs/src/host/cli/session.rs b/nvx-rs/src/host/cli/session.rs index f1aa16fa0..27d70032a 100644 --- a/nvx-rs/src/host/cli/session.rs +++ b/nvx-rs/src/host/cli/session.rs @@ -29,7 +29,6 @@ use std::time::Duration; use super::deadline::{Deadline, RETRY_INTERVAL, sleep}; use super::{Exit, Result, check_interrupt}; use crate::host::platform::{self, Transport}; -use crate::host::python::fs::os_error_message; use crate::host::python::path::PurePath; use crate::host::sandbox::options::validate_exec_environment; use crate::host::session::{ControlSession, SessionError}; @@ -212,7 +211,7 @@ impl Session { .raw_os_error() .is_some_and(|code| raised_at_once(code as u32)) { - return Err(Exit::error(os_error_message(&error, None))); + return Err(Exit::from_os_error(error)); } if deadline.passed() { return Err(Exit::error(format!( @@ -295,7 +294,7 @@ impl Session { if closed || closes(&error) { Exit::EndpointClosed } else { - Exit::error(os_error_message(&error, None)) + Exit::from_os_error(error) } } SessionError::Reset => Exit::error("managed control session was reset"),