Skip to content

Commit

Permalink
Detailed error tags for implicit io conversions
Browse files Browse the repository at this point in the history
Summary:
Implicit I/O and I/O category for serde_json are currently categorized very vaguely with just `IoSystem` and `Tier0` respectively. We already have code that categorizes `io::Error` more precisely so refactoring a bit.

`io_error_kind_tag` is currently duplicated with the one in `fs_util.rs`, the next diff will clean that up

Reviewed By: JakobDegen

Differential Revision: D69144749

fbshipit-source-id: 371d9296717ba2856fa7b68e8b0cfa1280b85001
  • Loading branch information
Will-MingLun-Li authored and facebook-github-bot committed Feb 5, 2025
1 parent dbf0c5b commit d609e6e
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 4 deletions.
1 change: 1 addition & 0 deletions app/buck2_error/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ rust_library(
"fbsource//third-party/rust:hex",
"fbsource//third-party/rust:http",
"fbsource//third-party/rust:hyper",
"fbsource//third-party/rust:libc",
"fbsource//third-party/rust:prost",
"fbsource//third-party/rust:prost-types",
"fbsource//third-party/rust:ref-cast",
Expand Down
1 change: 1 addition & 0 deletions app/buck2_error/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ fancy-regex = { workspace = true }
hex = { workspace = true }
http = { workspace = true }
hyper = { workspace = true }
libc = { workspace = true }
prost = { workspace = true }
prost-types = { workspace = true }
ref-cast = { workspace = true }
Expand Down
8 changes: 7 additions & 1 deletion app/buck2_error/src/conversion/serde_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@

use serde_json::error::Category;

use crate::conversion::stds::io::io_error_kind_to_error_tag;

impl From<serde_json::Error> for crate::Error {
#[cold]
#[track_caller]
fn from(value: serde_json::Error) -> Self {
let error_tag = match value.classify() {
Category::Data | Category::Syntax => crate::ErrorTag::Input,
Category::Eof | Category::Io => crate::ErrorTag::Tier0,
Category::Eof => crate::ErrorTag::Tier0,
Category::Io => match value.io_error_kind() {
Some(error_kind) => io_error_kind_to_error_tag(error_kind),
None => crate::ErrorTag::Tier0,
},
};

crate::conversion::from_any_with_tag(value, error_tag)
Expand Down
2 changes: 1 addition & 1 deletion app/buck2_error/src/conversion/stds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

mod array;
mod convert;
mod io;
pub(crate) mod io;
mod num;
mod path;
mod str;
Expand Down
50 changes: 48 additions & 2 deletions app/buck2_error/src/conversion/stds/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,56 @@
* of this source tree.
*/

impl From<std::io::Error> for crate::Error {
use std::io;

use buck2_data::error::ErrorTag;

// https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
// "The process cannot access the file because it is being used by another process."
const ERROR_SHARING_VIOLATION: i32 = 32;

pub(crate) fn io_error_kind_to_error_tag(kind: io::ErrorKind) -> ErrorTag {
match kind {
io::ErrorKind::NotFound => ErrorTag::IoNotFound,
io::ErrorKind::PermissionDenied => ErrorTag::IoPermissionDenied,
io::ErrorKind::TimedOut => ErrorTag::IoTimeout,
io::ErrorKind::ExecutableFileBusy => ErrorTag::IoExecutableFileBusy,
io::ErrorKind::BrokenPipe => ErrorTag::IoBrokenPipe,
io::ErrorKind::StorageFull => ErrorTag::IoStorageFull,
io::ErrorKind::ConnectionAborted => ErrorTag::IoConnectionAborted,
_ => ErrorTag::IoSystem,
}
}

fn io_error_kind_tag(e: &io::Error) -> ErrorTag {
let kind_tag = io_error_kind_to_error_tag(e.kind());
if kind_tag != ErrorTag::IoSystem {
return kind_tag;
}

if let Some(os_error_code) = e.raw_os_error() {
'from_os: {
let from_os = match os_error_code {
libc::ENOTCONN => ErrorTag::IoNotConnected,
libc::ECONNABORTED => ErrorTag::IoConnectionAborted,
_ => break 'from_os,
};
return from_os;
}

if cfg!(windows) && os_error_code == ERROR_SHARING_VIOLATION {
return ErrorTag::IoWindowsSharingViolation;
}
}

ErrorTag::IoSystem
}

impl From<io::Error> for crate::Error {
#[cold]
#[track_caller]
fn from(value: std::io::Error) -> Self {
crate::conversion::from_any_with_tag(value, crate::ErrorTag::IoSystem)
let error_tag = io_error_kind_tag(&value);
crate::conversion::from_any_with_tag(value, error_tag)
}
}

0 comments on commit d609e6e

Please sign in to comment.