Skip to content

Commit

Permalink
Migration to error stack, keeping old traced err for now
Browse files Browse the repository at this point in the history
  • Loading branch information
zakstucke committed Jan 22, 2024
1 parent 00e0ae3 commit 03615f3
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 46 deletions.
11 changes: 11 additions & 0 deletions rust/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 rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ shlex = '1.2'
spez = '0.1'
tracing = '0.1'
tracing-appender = '0.2'
error-stack = '0.4'

[dependencies.aide]
features = ['axum']
Expand Down
6 changes: 3 additions & 3 deletions rust/bitbazaar/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod tests {
use rstest::*;

use super::*;
use crate::errors::TracedErr;
use crate::{aer, errors::prelude::*};

#[rstest]
// <-- basics:
Expand Down Expand Up @@ -38,8 +38,8 @@ mod tests {
#[case] cmd_str: &str,
#[case] exp_std_all: S,
#[case] code: i32,
) -> Result<(), TracedErr> {
let res = run_cmd(cmd_str)?;
) -> Result<(), AnyErr> {
let res = aer!(run_cmd(cmd_str))?;
assert_eq!(res.code, code, "{}", res.std_all());
assert_eq!(res.std_all().trim(), exp_std_all.into());
Ok(())
Expand Down
23 changes: 20 additions & 3 deletions rust/bitbazaar/cli/run_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::errors::TracedResult;
use crate::errors::prelude::*;

/// The result of running a command
pub struct CmdOut {
Expand Down Expand Up @@ -28,6 +28,22 @@ impl CmdOut {
}
}

#[derive(Debug)]
pub enum CmdErr {
/// An arbitrary downstream error:
Unknown(String),
}

impl std::fmt::Display for CmdErr {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
CmdErr::Unknown(msg) => write!(f, "{}", msg),
}
}
}

impl error_stack::Context for CmdErr {}

/// Run a dynamic shell command and return the output.
///
/// WARNING: this opens up the possibility of dependency injection attacks, so should only be used when the command is trusted.
Expand All @@ -38,12 +54,13 @@ impl CmdOut {
/// - `||` or
/// - `|` pipe
/// - `~` home dir
pub fn run_cmd<S: Into<String>>(cmd_str: S) -> TracedResult<CmdOut> {
pub fn run_cmd<S: Into<String>>(cmd_str: S) -> Result<CmdOut, CmdErr> {
let (code, output, error) = run_script::run(
cmd_str.into().as_str(),
&vec![],
&run_script::ScriptOptions::new(),
)?;
)
.map_err(|e| CmdErr::Unknown(e.to_string()))?;

Ok(CmdOut {
stdout: output,
Expand Down
13 changes: 13 additions & 0 deletions rust/bitbazaar/errors/any.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use error_stack::Context;

/// A generic trace_stack error to use when you don't want to create custom error types.
#[derive(Debug, Default)]
pub struct AnyErr;

impl std::fmt::Display for AnyErr {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "AnyErr")
}
}

impl Context for AnyErr {}
85 changes: 85 additions & 0 deletions rust/bitbazaar/errors/macros.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,88 @@
#[macro_export]
/// A helper for the aer! macro.
macro_rules! _aer_inner {
($any_err:expr) => {{
use error_stack::{Context, Report, Result, ResultExt};

$crate::spez! {
for x = $any_err;

// error_stack err -> Report<AnyErr>
match<T: error_stack::Context> T -> Report<AnyErr> {
Report::new(x).change_context(AnyErr)
}
// Error + Sync + Send + 'static -> Report<AnyErr>
match<T: std::error::Error + Sync + Send + 'static> T -> Report<AnyErr> {
Report::new(x).change_context(AnyErr)
}
// Report<T> -> Report<AnyErr>
match<T> Report<T> -> Report<AnyErr> {
x.change_context(AnyErr)
}
// error_stack::Result<T, E> -> Result<T, AnyErr>
match<T, E: Context> Result<T, E> -> Result<T, AnyErr> {
x.change_context(AnyErr)
}
// core::result::Result<T, E> -> Result<T, AnyErr>
match<T, E: Context> core::result::Result<T, E> -> Result<T, AnyErr> {
x.change_context(AnyErr)
}
// Into<String> -> Report<AnyErr>
match<T: Into<String>> T -> Report<AnyErr> {
Report::new(AnyErr).attach_printable(x.into())
}
}
}};
}

#[macro_export]
/// A helper for the aer! macro.
macro_rules! _aer_inner_with_txt {
($err_or_result_or_str:expr, $str:expr) => {{
use error_stack::{Context, Report, Result, ResultExt};
use $crate::_aer_inner;

let foo = _aer_inner!($err_or_result_or_str);
$crate::spez! {
for x = foo;

// Not lazy if already a report:
match<T> Report<T> -> Report<T> {
x.attach_printable($str)
}

// Otherwise, lazy:
match<T, E: Context> Result<T, E> -> Result<T, E> {
x.attach_printable($str)
}
}
}};
}

/// A macro for building `AnyErr` reports easily from other error_stack errors, other reports and base errors outside of error_stack.
#[macro_export]
macro_rules! aer {
() => {{
use error_stack::Report;
Report::new(AnyErr)
}};

($err_or_result_or_str:expr) => {{
use $crate::_aer_inner;
_aer_inner!($err_or_result_or_str)
}};

($err_or_result_or_str:expr, $str:expr) => {{
use $crate::_aer_inner_with_txt;
_aer_inner_with_txt!($err_or_result_or_str, $str)
}};

($err_or_result_or_str:expr, $str:expr, $($arg:expr),*) => {{
use $crate::_aer_inner_with_txt;
_aer_inner_with_txt!($err_or_result_or_str, format!($str, $($arg),*))
}};
}

/// A macro for creating a TracedErr from a string or another existing error.
#[macro_export]
macro_rules! err {
Expand Down
9 changes: 9 additions & 0 deletions rust/bitbazaar/errors/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod any;
mod generic_err;
mod macros;
mod test_errs;
Expand All @@ -7,6 +8,14 @@ mod traced_error;
pub use traced_error::set_axum_debug;
pub use traced_error::{TracedErr, TracedResult};

pub(crate) mod prelude {
pub use error_stack::{bail, report, Result, ResultExt};

pub use super::any::AnyErr;
#[allow(unused_imports)]
pub use crate::aer;
}

#[cfg(test)]
mod tests {
use colored::Colorize;
Expand Down
27 changes: 16 additions & 11 deletions rust/bitbazaar/logging/create_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ use std::{path::PathBuf, str::FromStr};

use once_cell::sync::Lazy;
use parking_lot::Mutex;
use tonic::metadata::{Ascii, MetadataValue};
use tracing::{Dispatch, Level, Metadata, Subscriber};
use tracing_appender::non_blocking::WorkerGuard;
use tracing_subscriber::{
filter::FilterFn, fmt::MakeWriter, prelude::*, registry::LookupSpan, Layer,
};

use crate::{err, errors::TracedErr};
use crate::errors::prelude::*;

/// Specify which logs should be matched by this layer.
///
Expand Down Expand Up @@ -235,7 +236,7 @@ impl CreatedSubscriber {
/// }]).unwrap();
/// sub.into_global(); // Register it as the global logger, this can only be done once
/// ```
pub fn create_subscriber(layers: Vec<SubLayer>) -> Result<CreatedSubscriber, TracedErr> {
pub fn create_subscriber(layers: Vec<SubLayer>) -> Result<CreatedSubscriber, AnyErr> {
let all_loc_matchers = layers
.iter()
.filter_map(|target| target.loc_matcher.clone())
Expand Down Expand Up @@ -263,15 +264,15 @@ pub fn create_subscriber(layers: Vec<SubLayer>) -> Result<CreatedSubscriber, Tra
SubLayerVariant::File { file_prefix, dir } => {
// Throw if dir is an existing file:
if dir.is_file() {
return Err(err!(
bail!(report!(AnyErr).attach_printable(format!(
"Log directory is an existing file: {}",
dir.to_string_lossy()
));
)));
}

// Create the dir if missing:
if !dir.exists() {
std::fs::create_dir_all(&dir)?;
std::fs::create_dir_all(&dir).change_context(AnyErr)?;
}

// Rotate the file daily:
Expand Down Expand Up @@ -306,8 +307,10 @@ pub fn create_subscriber(layers: Vec<SubLayer>) -> Result<CreatedSubscriber, Tra
let mut header_map = tonic::metadata::MetadataMap::new();
for (key, value) in headers {
header_map.insert(
tonic::metadata::MetadataKey::from_str(&key)?,
value.parse()?,
tonic::metadata::MetadataKey::from_str(&key).change_context(AnyErr)?,
value
.parse::<MetadataValue<Ascii>>()
.change_context(AnyErr)?,
);
}

Expand All @@ -320,7 +323,8 @@ pub fn create_subscriber(layers: Vec<SubLayer>) -> Result<CreatedSubscriber, Tra
.with_endpoint(endpoint)
.with_metadata(header_map),
)
.install_batch(opentelemetry_sdk::runtime::Tokio)?;
.install_batch(opentelemetry_sdk::runtime::Tokio)
.change_context(AnyErr)?;
let layer = tracing_opentelemetry::layer().with_tracer(tracer);
layer.boxed()
}
Expand All @@ -345,7 +349,7 @@ fn filter_layer(
filter: SubLayerFilter,
loc_matcher: Option<regex::Regex>,
all_loc_matchers: &[regex::Regex],
) -> Result<FilterFn<impl Fn(&Metadata<'_>) -> bool>, TracedErr> {
) -> Result<FilterFn<impl Fn(&Metadata<'_>) -> bool>, AnyErr> {
// Needs to be a vec to pass through to the filter fn:
let all_loc_matchers = all_loc_matchers.to_vec();

Expand Down Expand Up @@ -395,7 +399,7 @@ fn create_fmt_layer<S, W>(
include_loc: bool,
include_color: bool,
writer: W,
) -> Result<Box<dyn Layer<S> + Send + Sync + 'static>, TracedErr>
) -> Result<Box<dyn Layer<S> + Send + Sync + 'static>, AnyErr>
where
S: Subscriber,
for<'a> S: LookupSpan<'a>, // Each layer has a different type, so have to box for return
Expand All @@ -415,7 +419,8 @@ where
// also no need for any more than ms precision,
// also make it a UTC time:
let timer =
time::format_description::parse("[hour]:[minute]:[second].[subsecond digits:3]")?;
time::format_description::parse("[hour]:[minute]:[second].[subsecond digits:3]")
.change_context(AnyErr)?;
let time_offset = time::UtcOffset::current_local_offset().unwrap_or(time::UtcOffset::UTC);
let timer = tracing_subscriber::fmt::time::OffsetTime::new(time_offset, timer);

Expand Down
Loading

0 comments on commit 03615f3

Please sign in to comment.