extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-08-10 06:48:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

Refactor: Construct IoError from std::io::Error instead of std::io::ErrorKind (#15777)

Browse Source
This commit is contained in:
Piepmatz
2025-05-18 14:52:40 +02:00
committed by GitHub
parent c4dcfdb77b
commit 833471241a
80 changed files with 408 additions and 299 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
crates/nu-protocol/Cargo.toml
View File

@ -51,6 +51,7 @@ nix = { workspace = true, default-features = false, features = ["signal"] }
[target.'cfg(windows)'.dependencies]
dirs-sys = { workspace = true }
windows-sys = { workspace = true }
windows = { workspace = true }
[features]
default = ["os"]

6
crates/nu-protocol/src/engine/engine_state.rs
View File

@ -353,7 +353,7 @@ impl EngineState {
let cwd = self.cwd(Some(stack))?;
std::env::set_current_dir(cwd).map_err(|err| {
IoError::new_internal(err.kind(), "Could not set current dir", crate::location!())
IoError::new_internal(err, "Could not set current dir", crate::location!())
})?;
if let Some(config) = stack.config.take() {
@ -546,7 +546,7 @@ impl EngineState {
Ok(PluginRegistryFile::default())
} else {
Err(ShellError::Io(IoError::new_internal_with_path(
err.kind(),
err,
"Failed to open plugin file",
crate::location!(),
PathBuf::from(plugin_path),
@ -563,7 +563,7 @@ impl EngineState {
// Write it to the same path
let plugin_file = File::create(plugin_path.as_path()).map_err(|err| {
IoError::new_internal_with_path(
err.kind(),
err,
"Failed to write plugin file",
crate::location!(),
PathBuf::from(plugin_path),

4
crates/nu-protocol/src/errors/labeled_error.rs
View File

@ -143,11 +143,11 @@ impl LabeledError {
/// [`ShellError`] implements `miette::Diagnostic`:
///
/// ```rust
/// # use nu_protocol::{ShellError, LabeledError, shell_error::io::IoError, Span};
/// # use nu_protocol::{ShellError, LabeledError, shell_error::{self, io::IoError}, Span};
/// #
/// let error = LabeledError::from_diagnostic(
/// &ShellError::Io(IoError::new_with_additional_context(
/// std::io::ErrorKind::Other,
/// shell_error::io::ErrorKind::from_std(std::io::ErrorKind::Other),
/// Span::test_data(),
/// None,
/// "some error"

171
crates/nu-protocol/src/errors/shell_error/io.rs
View File

@ -38,7 +38,7 @@ use super::{ShellError, location::Location};
///
/// # let span = Span::test_data();
/// let path = PathBuf::from("/some/missing/file");
/// let error = IoError::new(std::io::ErrorKind::NotFound, span, path);
/// let error = IoError::new(ErrorKind::FileNotFound, span, path);
/// println!("Error: {:?}", error);
/// ```
///
@ -47,7 +47,7 @@ use super::{ShellError, location::Location};
/// # use nu_protocol::shell_error::io::{IoError, ErrorKind};
// #
/// let error = IoError::new_internal(
/// std::io::ErrorKind::UnexpectedEof,
/// ErrorKind::from_std(std::io::ErrorKind::UnexpectedEof),
/// "Failed to read data from buffer",
/// nu_protocol::location!()
/// );
@ -93,7 +93,7 @@ pub struct IoError {
/// and is part of [`std::io::Error`].
/// If a kind cannot be represented by it, consider adding a new variant to [`ErrorKind`].
///
/// Only in very rare cases should [`std::io::ErrorKind::Other`] be used, make sure you provide
/// Only in very rare cases should [`std::io::ErrorKind::other()`] be used, make sure you provide
/// `additional_context` to get useful errors in these cases.
pub kind: ErrorKind,
@ -125,15 +125,63 @@ pub struct IoError {
pub location: Option<String>,
}
/// Prevents other crates from constructing certain enum variants directly.
///
/// This type is only used to block construction while still allowing pattern matching.
/// It's not meant to be used for anything else.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
struct Sealed;
#[derive(Debug, Clone, Copy, PartialEq, Eq, Diagnostic)]
pub enum ErrorKind {
Std(std::io::ErrorKind),
/// [`std::io::ErrorKind`] from the standard library.
///
/// This variant wraps a standard library error kind and extends our own error enum with it.
/// The hidden field prevents other crates, even our own, from constructing this directly.
/// Most of the time, you already have a full [`std::io::Error`], so just pass that directly to
/// [`IoError::new`] or [`IoError::new_with_additional_context`].
/// This allows us to inspect the raw os error of `std::io::Error`s.
///
/// Matching is still easy:
///
/// ```rust
/// # use nu_protocol::shell_error::io::ErrorKind;
/// #
/// # let err_kind = ErrorKind::from_std(std::io::ErrorKind::NotFound);
/// match err_kind {
/// ErrorKind::Std(std::io::ErrorKind::NotFound, ..) => { /* ... */ }
/// _ => {}
/// }
/// ```
///
/// If you want to provide an [`std::io::ErrorKind`] manually, use [`ErrorKind::from_std`].
#[allow(private_interfaces)]
Std(std::io::ErrorKind, Sealed),
NotAFile,
/// The file or directory is in use by another program.
///
/// On Windows, this maps to
/// [`ERROR_SHARING_VIOLATION`](windows::Win32::Foundation::ERROR_SHARING_VIOLATION) and
/// prevents access like deletion or modification.
AlreadyInUse,
// use these variants in cases where we know precisely whether a file or directory was expected
FileNotFound,
DirectoryNotFound,
}
impl ErrorKind {
/// Construct an [`ErrorKind`] from a [`std::io::ErrorKind`] without a full [`std::io::Error`].
///
/// In most cases, you should use [`IoError::new`] and pass the full [`std::io::Error`] instead.
/// This method is only meant for cases where we provide our own io error kinds.
pub fn from_std(kind: std::io::ErrorKind) -> Self {
Self::Std(kind, Sealed)
}
}
#[derive(Debug, Clone, PartialEq, Eq, Error, Diagnostic)]
#[error("{0}")]
pub struct AdditionalContext(String);
@ -237,10 +285,10 @@ impl IoError {
///
/// # Examples
/// ```rust
/// use nu_protocol::shell_error::io::IoError;
/// use nu_protocol::shell_error::{self, io::IoError};
///
/// let error = IoError::new_internal(
/// std::io::ErrorKind::UnexpectedEof,
/// shell_error::io::ErrorKind::from_std(std::io::ErrorKind::UnexpectedEof),
/// "Failed to read from buffer",
/// nu_protocol::location!(),
/// );
@ -268,11 +316,11 @@ impl IoError {
///
/// # Examples
/// ```rust
/// use nu_protocol::shell_error::io::IoError;
/// use nu_protocol::shell_error::{self, io::IoError};
/// use std::path::PathBuf;
///
/// let error = IoError::new_internal_with_path(
/// std::io::ErrorKind::NotFound,
/// shell_error::io::ErrorKind::FileNotFound,
/// "Could not find special file",
/// nu_protocol::location!(),
/// PathBuf::from("/some/file"),
@ -297,14 +345,37 @@ impl IoError {
///
/// This method is particularly useful when you need to handle multiple I/O errors which all
/// take the same span and path.
/// Instead of calling `.map_err(|err| IoError::new(err.kind(), span, path))` every time, you
/// Instead of calling `.map_err(|err| IoError::new(err, span, path))` every time, you
/// can create the factory closure once and pass that into `.map_err`.
pub fn factory<'p, P>(span: Span, path: P) -> impl Fn(std::io::Error) -> Self + use<'p, P>
where
P: Into<Option<&'p Path>>,
{
let path = path.into();
move |err: std::io::Error| IoError::new(err.kind(), span, path.map(PathBuf::from))
move |err: std::io::Error| IoError::new(err, span, path.map(PathBuf::from))
}
}
impl From<std::io::Error> for ErrorKind {
fn from(err: std::io::Error) -> Self {
(&err).into()
}
}
impl From<&std::io::Error> for ErrorKind {
fn from(err: &std::io::Error) -> Self {
#[cfg(windows)]
if let Some(raw_os_error) = err.raw_os_error() {
use windows::Win32::Foundation;
#[allow(clippy::single_match, reason = "in the future we can expand here")]
match Foundation::WIN32_ERROR(raw_os_error as u32) {
Foundation::ERROR_SHARING_VIOLATION => return ErrorKind::AlreadyInUse,
_ => {}
}
}
ErrorKind::Std(err.kind(), Sealed)
}
}
@ -312,7 +383,7 @@ impl StdError for IoError {}
impl Display for IoError {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self.kind {
ErrorKind::Std(std::io::ErrorKind::NotFound) => write!(f, "Not found"),
ErrorKind::Std(std::io::ErrorKind::NotFound, _) => write!(f, "Not found"),
ErrorKind::FileNotFound => write!(f, "File not found"),
ErrorKind::DirectoryNotFound => write!(f, "Directory not found"),
_ => write!(f, "I/O error"),
@ -323,13 +394,14 @@ impl Display for IoError {
impl Display for ErrorKind {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
ErrorKind::Std(std::io::ErrorKind::NotFound) => write!(f, "Not found"),
ErrorKind::Std(error_kind) => {
ErrorKind::Std(std::io::ErrorKind::NotFound, _) => write!(f, "Not found"),
ErrorKind::Std(error_kind, _) => {
let msg = error_kind.to_string();
let (first, rest) = msg.split_at(1);
write!(f, "{}{}", first.to_uppercase(), rest)
}
ErrorKind::NotAFile => write!(f, "Not a file"),
ErrorKind::AlreadyInUse => write!(f, "Already in use"),
ErrorKind::FileNotFound => write!(f, "File not found"),
ErrorKind::DirectoryNotFound => write!(f, "Directory not found"),
}
@ -342,7 +414,7 @@ impl Diagnostic for IoError {
fn code<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> {
let mut code = String::from("nu::shell::io::");
match self.kind {
ErrorKind::Std(error_kind) => match error_kind {
ErrorKind::Std(error_kind, _) => match error_kind {
std::io::ErrorKind::NotFound => code.push_str("not_found"),
std::io::ErrorKind::PermissionDenied => code.push_str("permission_denied"),
std::io::ErrorKind::ConnectionRefused => code.push_str("connection_refused"),
@ -366,6 +438,7 @@ impl Diagnostic for IoError {
kind => code.push_str(&kind.to_string().to_lowercase().replace(" ", "_")),
},
ErrorKind::NotAFile => code.push_str("not_a_file"),
ErrorKind::AlreadyInUse => code.push_str("already_in_use"),
ErrorKind::FileNotFound => code.push_str("file_not_found"),
ErrorKind::DirectoryNotFound => code.push_str("directory_not_found"),
}
@ -378,7 +451,10 @@ impl Diagnostic for IoError {
let path = format!("'{}'", path.display());
match self.kind {
ErrorKind::NotAFile => format!("{path} is not a file"),
ErrorKind::Std(std::io::ErrorKind::NotFound)
ErrorKind::AlreadyInUse => {
format!("{path} is already being used by another program")
}
ErrorKind::Std(std::io::ErrorKind::NotFound, _)
| ErrorKind::FileNotFound
| ErrorKind::DirectoryNotFound => format!("{path} does not exist"),
_ => format!("The error occurred at {path}"),
@ -430,16 +506,10 @@ impl From<IoError> for std::io::Error {
}
}
impl From<std::io::ErrorKind> for ErrorKind {
fn from(value: std::io::ErrorKind) -> Self {
ErrorKind::Std(value)
}
}
impl From<ErrorKind> for std::io::ErrorKind {
fn from(value: ErrorKind) -> Self {
match value {
ErrorKind::Std(error_kind) => error_kind,
ErrorKind::Std(error_kind, _) => error_kind,
_ => std::io::ErrorKind::Other,
}
}
@ -455,8 +525,8 @@ pub enum NotFound {
Directory,
}
/// Extension trait for working with [`std::io::ErrorKind`].
pub trait ErrorKindExt {
/// Extension trait for working with [`std::io::Error`].
pub trait IoErrorExt {
/// Map [`NotFound`](std::io::ErrorKind) variants into more precise variants.
///
/// The OS doesn't know when an entity was not found whether it was meant to be a file or a
@ -469,7 +539,7 @@ pub trait ErrorKindExt {
/// If the file isn't found, return [`FileNotFound`](ErrorKind::FileNotFound).
/// ```rust
/// # use nu_protocol::{
/// # shell_error::io::{ErrorKind, ErrorKindExt, IoError, NotFound},
/// # shell_error::io::{ErrorKind, IoErrorExt, IoError, NotFound},
/// # ShellError, Span,
/// # };
/// # use std::{fs, path::PathBuf};
@ -479,7 +549,7 @@ pub trait ErrorKindExt {
/// let a_file = PathBuf::from("scripts/ellie.nu");
/// let ellie = fs::read_to_string(&a_file).map_err(|err| {
/// ShellError::Io(IoError::new(
/// err.kind().not_found_as(NotFound::File),
/// err.not_found_as(NotFound::File),
/// span,
/// a_file,
/// ))
@ -498,21 +568,46 @@ pub trait ErrorKindExt {
fn not_found_as(self, kind: NotFound) -> ErrorKind;
}
impl ErrorKindExt for std::io::ErrorKind {
impl IoErrorExt for ErrorKind {
fn not_found_as(self, kind: NotFound) -> ErrorKind {
match (kind, self) {
(NotFound::File, Self::NotFound) => ErrorKind::FileNotFound,
(NotFound::Directory, Self::NotFound) => ErrorKind::DirectoryNotFound,
_ => ErrorKind::Std(self),
}
}
}
impl ErrorKindExt for ErrorKind {
fn not_found_as(self, kind: NotFound) -> ErrorKind {
match self {
Self::Std(std_kind) => std_kind.not_found_as(kind),
(NotFound::File, Self::Std(std::io::ErrorKind::NotFound, _)) => ErrorKind::FileNotFound,
(NotFound::Directory, Self::Std(std::io::ErrorKind::NotFound, _)) => {
ErrorKind::DirectoryNotFound
}
_ => self,
}
}
}
impl IoErrorExt for std::io::Error {
fn not_found_as(self, kind: NotFound) -> ErrorKind {
ErrorKind::from(self).not_found_as(kind)
}
}
impl IoErrorExt for &std::io::Error {
fn not_found_as(self, kind: NotFound) -> ErrorKind {
ErrorKind::from(self).not_found_as(kind)
}
}
#[cfg(test)]
mod assert_not_impl {
use super::*;
/// Assertion that `ErrorKind` does not implement `From<std::io::ErrorKind>`.
///
/// This implementation exists only in tests to make sure that no crate,
/// including ours, accidentally adds a `From<std::io::ErrorKind>` impl for `ErrorKind`.
/// If someone tries, it will fail due to conflicting implementations.
///
/// We want to force usage of [`IoError::new`] with a full [`std::io::Error`] instead of
/// allowing conversion from just an [`std::io::ErrorKind`].
/// That way, we can properly inspect and classify uncategorized I/O errors.
impl From<std::io::ErrorKind> for ErrorKind {
fn from(_: std::io::ErrorKind) -> Self {
unimplemented!("ErrorKind should not implement From<std::io::ErrorKind>")
}
}
}

16
crates/nu-protocol/src/pipeline/byte_stream.rs
View File

@ -246,7 +246,7 @@ impl ByteStream {
if let Some(mut reader) = self.reader() {
// Copy the number of skipped bytes into the sink before proceeding
io::copy(&mut (&mut reader).take(n), &mut io::sink())
.map_err(|err| IoError::new(err.kind(), span, None))?;
.map_err(|err| IoError::new(err, span, None))?;
Ok(
ByteStream::read(reader, span, Signals::empty(), ByteStreamType::Binary)
.with_known_size(known_size),
@ -367,7 +367,7 @@ impl ByteStream {
/// binary.
#[cfg(feature = "os")]
pub fn stdin(span: Span) -> Result<Self, ShellError> {
let stdin = os_pipe::dup_stdin().map_err(|err| IoError::new(err.kind(), span, None))?;
let stdin = os_pipe::dup_stdin().map_err(|err| IoError::new(err, span, None))?;
let source = ByteStreamSource::File(convert_file(stdin));
Ok(Self::new(
source,
@ -853,7 +853,7 @@ impl Iterator for Lines {
trim_end_newline(&mut string);
Some(Ok(string))
}
Err(e) => Some(Err(IoError::new(e.kind(), self.span, None).into())),
Err(err) => Some(Err(IoError::new(err, self.span, None).into())),
}
}
}
@ -1052,7 +1052,7 @@ impl Iterator for SplitRead {
self.internal.next().map(|r| {
r.map_err(|err| {
ShellError::Io(IoError::new_internal(
err.kind(),
err,
"Could not get next value for SplitRead",
crate::location!(),
))
@ -1094,7 +1094,7 @@ impl Chunks {
fn next_string(&mut self) -> Result<Option<String>, (Vec<u8>, ShellError)> {
let from_io_error = |err: std::io::Error| match ShellErrorBridge::try_from(err) {
Ok(err) => err.0,
Err(err) => IoError::new(err.kind(), self.span, None).into(),
Err(err) => IoError::new(err, self.span, None).into(),
};
// Get some data from the reader
@ -1177,11 +1177,7 @@ impl Iterator for Chunks {
Ok(buf) => buf,
Err(err) => {
self.error = true;
return Some(Err(ShellError::Io(IoError::new(
err.kind(),
self.span,
None,
))));
return Some(Err(ShellError::Io(IoError::new(err, self.span, None))));
}
};
if !buf.is_empty() {

18
crates/nu-protocol/src/pipeline/pipeline_data.rs
View File

@ -222,14 +222,14 @@ impl PipelineData {
let bytes = value_to_bytes(value)?;
dest.write_all(&bytes).map_err(|err| {
IoError::new_internal(
err.kind(),
err,
"Could not write PipelineData to dest",
crate::location!(),
)
})?;
dest.flush().map_err(|err| {
IoError::new_internal(
err.kind(),
err,
"Could not flush PipelineData to dest",
crate::location!(),
)
@ -241,14 +241,14 @@ impl PipelineData {
let bytes = value_to_bytes(value)?;
dest.write_all(&bytes).map_err(|err| {
IoError::new_internal(
err.kind(),
err,
"Could not write PipelineData to dest",
crate::location!(),
)
})?;
dest.write_all(b"\n").map_err(|err| {
IoError::new_internal(
err.kind(),
err,
"Could not write linebreak after PipelineData to dest",
crate::location!(),
)
@ -256,7 +256,7 @@ impl PipelineData {
}
dest.flush().map_err(|err| {
IoError::new_internal(
err.kind(),
err,
"Could not flush PipelineData to dest",
crate::location!(),
)
@ -775,11 +775,9 @@ where
let io_error_map = |err: std::io::Error, location: Location| {
let context = format!("Writing to {} failed", destination_name);
match span {
None => IoError::new_internal(err.kind(), context, location),
Some(span) if span == Span::unknown() => {
IoError::new_internal(err.kind(), context, location)
}
Some(span) => IoError::new_with_additional_context(err.kind(), span, None, context),
None => IoError::new_internal(err, context, location),
Some(span) if span == Span::unknown() => IoError::new_internal(err, context, location),
Some(span) => IoError::new_with_additional_context(err, span, None, context),
}
};

6
crates/nu-protocol/src/process/child.rs
View File

@ -81,7 +81,7 @@ impl ExitStatusFuture {
}
Ok(Ok(status)) => Ok(status),
Ok(Err(err)) => Err(ShellError::Io(IoError::new_with_additional_context(
err.kind(),
err,
span,
None,
"failed to get exit code",
@ -276,7 +276,7 @@ impl ChildProcess {
})
.map_err(|err| {
IoError::new_with_additional_context(
err.kind(),
err,
span,
None,
"Could now spawn exit status waiter",
@ -325,7 +325,7 @@ impl ChildProcess {
}
let bytes = if let Some(stdout) = self.stdout {
collect_bytes(stdout).map_err(|err| IoError::new(err.kind(), self.span, None))?
collect_bytes(stdout).map_err(|err| IoError::new(err, self.span, None))?
} else {
Vec::new()
};