Skip to content

Commit

Permalink
Add typed errors with snafu (#3832)
Browse files Browse the repository at this point in the history
  • Loading branch information
casey authored Jun 27, 2024
1 parent 6103de9 commit 18bbbd2
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 29 deletions.
22 changes: 22 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ serde_json = { version = "1.0.81", features = ["preserve_order"] }
serde_with = "3.7.0"
serde_yaml = "0.9.17"
sha3 = "0.10.8"
snafu = "0.8.3"
sysinfo = "0.30.3"
tempfile = "3.2.0"
tokio = { version = "1.17.0", features = ["rt-multi-thread"] }
Expand Down
23 changes: 12 additions & 11 deletions src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,30 @@ pub(crate) struct Arguments {
}

impl Arguments {
pub(crate) fn run(self) -> SubcommandResult {
pub(crate) fn run(self) -> SnafuResult<Option<Box<dyn subcommand::Output>>> {
let mut env: BTreeMap<String, String> = BTreeMap::new();

for (var, value) in env::vars_os() {
let Some(var) = var.to_str() else {
for (variable, value) in env::vars_os() {
let Some(variable) = variable.to_str() else {
continue;
};

let Some(key) = var.strip_prefix("ORD_") else {
let Some(key) = variable.strip_prefix("ORD_") else {
continue;
};

env.insert(
key.into(),
value.into_string().map_err(|value| {
anyhow!(
"environment variable `{var}` not valid unicode: `{}`",
value.to_string_lossy()
)
})?,
value
.into_string()
.map_err(|value| SnafuError::EnvVarUnicode {
backtrace: Backtrace::capture(),
value,
variable: variable.into(),
})?,
);
}

self.subcommand.run(Settings::load(self.options)?)
Ok(self.subcommand.run(Settings::load(self.options)?)?)
}
}
66 changes: 66 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use super::*;

#[derive(Debug, Snafu)]
#[snafu(context(suffix(false)), visibility(pub(crate)))]
pub(crate) enum SnafuError {
#[snafu(display("{err}"))]
Anyhow { err: anyhow::Error },
#[snafu(display("environment variable `{variable}` not valid unicode: `{}`", value.to_string_lossy()))]
EnvVarUnicode {
backtrace: Backtrace,
value: OsString,
variable: String,
},
#[snafu(display("I/O error at `{}`", path.display()))]
Io {
backtrace: Backtrace,
path: PathBuf,
source: io::Error,
},
}

impl From<Error> for SnafuError {
fn from(err: Error) -> SnafuError {
Self::Anyhow { err }
}
}

/// We currently use `anyhow` for error handling but are migrating to typed
/// errors using `snafu`. This trait exists to provide access to
/// `snafu::ResultExt::{context, with_context}`, which are otherwise shadowed
/// by `anhow::Context::{context, with_context}`. Once the migration is
/// complete, this trait can be deleted, and `snafu::ResultExt` used directly.
pub(crate) trait ResultExt<T, E>: Sized {
fn snafu_context<C, E2>(self, context: C) -> Result<T, E2>
where
C: snafu::IntoError<E2, Source = E>,
E2: std::error::Error + snafu::ErrorCompat;

#[allow(unused)]
fn with_snafu_context<F, C, E2>(self, context: F) -> Result<T, E2>
where
F: FnOnce(&mut E) -> C,
C: snafu::IntoError<E2, Source = E>,
E2: std::error::Error + snafu::ErrorCompat;
}

impl<T, E> ResultExt<T, E> for std::result::Result<T, E> {
fn snafu_context<C, E2>(self, context: C) -> Result<T, E2>
where
C: snafu::IntoError<E2, Source = E>,
E2: std::error::Error + snafu::ErrorCompat,
{
use snafu::ResultExt;
self.context(context)
}

fn with_snafu_context<F, C, E2>(self, context: F) -> Result<T, E2>
where
F: FnOnce(&mut E) -> C,
C: snafu::IntoError<E2, Source = E>,
E2: std::error::Error + snafu::ErrorCompat,
{
use snafu::ResultExt;
self.with_context(context)
}
}
9 changes: 3 additions & 6 deletions src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,9 @@ impl Index {

let path = settings.index().to_owned();

if let Err(err) = fs::create_dir_all(path.parent().unwrap()) {
bail!(
"failed to create data dir `{}`: {err}",
path.parent().unwrap().display()
);
}
let data_dir = path.parent().unwrap();

fs::create_dir_all(data_dir).snafu_context(error::Io { path: data_dir })?;

let index_cache_size = settings.index_cache_size();

Expand Down
48 changes: 39 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use {
chrono::{DateTime, TimeZone, Utc},
ciborium::Value,
clap::{ArgGroup, Parser},
error::{ResultExt, SnafuError},
html_escaper::{Escape, Trusted},
http::HeaderMap,
lazy_static::lazy_static,
Expand All @@ -58,10 +59,13 @@ use {
reqwest::Url,
serde::{Deserialize, Deserializer, Serialize},
serde_with::{DeserializeFromStr, SerializeDisplay},
snafu::{Backtrace, ErrorCompat, Snafu},
std::{
backtrace::BacktraceStatus,
cmp::{self, Reverse},
collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque},
env,
ffi::OsString,
fmt::{self, Display, Formatter},
fs,
io::{self, Cursor, Read},
Expand Down Expand Up @@ -104,6 +108,7 @@ mod blocktime;
pub mod chain;
pub mod decimal;
mod deserialize_from_str;
mod error;
mod fee_rate;
pub mod index;
mod inscriptions;
Expand All @@ -122,6 +127,7 @@ pub mod templates;
pub mod wallet;

type Result<T = (), E = Error> = std::result::Result<T, E>;
type SnafuResult<T = (), E = SnafuError> = std::result::Result<T, E>;

const TARGET_POSTAGE: Amount = Amount::from_sat(10_000);

Expand Down Expand Up @@ -260,15 +266,39 @@ pub fn main() {
match args.run() {
Err(err) => {
eprintln!("error: {err}");
err
.chain()
.skip(1)
.for_each(|cause| eprintln!("because: {cause}"));
if env::var_os("RUST_BACKTRACE")
.map(|val| val == "1")
.unwrap_or_default()
{
eprintln!("{}", err.backtrace());

if let SnafuError::Anyhow { err } = err {
for (i, err) in err.chain().skip(1).enumerate() {
if i == 0 {
eprintln!();
eprintln!("because:");
}

eprintln!("- {err}");
}

if env::var_os("RUST_BACKTRACE")
.map(|val| val == "1")
.unwrap_or_default()
{
eprintln!("{}", err.backtrace());
}
} else {
for (i, err) in err.iter_chain().skip(1).enumerate() {
if i == 0 {
eprintln!();
eprintln!("because:");
}

eprintln!("- {err}");
}

if let Some(backtrace) = err.backtrace() {
if backtrace.status() == BacktraceStatus::Captured {
eprintln!("backtrace:");
eprintln!("{backtrace}");
}
}
}

gracefully_shut_down_indexer();
Expand Down
4 changes: 2 additions & 2 deletions tests/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ fn config_invalid_error_message() {
fs::write(&config, "foo").unwrap();

CommandBuilder::new(format!("--config {} settings", config.to_str().unwrap()))
.stderr_regex("error: failed to deserialize config file `.*ord.yaml`\nbecause:.*")
.stderr_regex("error: failed to deserialize config file `.*ord.yaml`\n\nbecause:.*")
.expected_exit_code(1)
.run_and_extract_stdout();
}
Expand All @@ -77,7 +77,7 @@ fn config_not_found_error_message() {
let config = tempdir.path().join("ord.yaml");

CommandBuilder::new(format!("--config {} settings", config.to_str().unwrap()))
.stderr_regex("error: failed to open config file `.*ord.yaml`\nbecause:.*")
.stderr_regex("error: failed to open config file `.*ord.yaml`\n\nbecause:.*")
.expected_exit_code(1)
.run_and_extract_stdout();
}
Expand Down
2 changes: 1 addition & 1 deletion tests/wallet/sats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,6 @@ fn sats_from_tsv_file_not_found() {
.core(&core)
.ord(&ord)
.expected_exit_code(1)
.stderr_regex("error: I/O error reading `.*`\nbecause: .*\n")
.stderr_regex("error: I/O error reading `.*`\n\nbecause:.*")
.run_and_extract_stdout();
}

0 comments on commit 18bbbd2

Please sign in to comment.