Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error-stack support Debug hooks in no-std contexts #1556

Merged
merged 10 commits into from
Nov 30, 2022
1 change: 1 addition & 0 deletions packages/libs/error-stack/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ All notable changes to `error-stack` will be documented in this file.
### Features

- Add serializing support using [`serde`](https://serde.rs) ([#1290](https://github.com/hashintel/hash/pull/1290))
- Support `Debug` hooks on `no-std` platforms via the `hooks` feature ([#1556](https://github.com/hashintel/hash/pull/1556))

## [0.2.4](https://github.com/hashintel/hash/tree/error-stack%400.2.4/packages/libs/error-stack) - 2022-11-04

Expand Down
2 changes: 2 additions & 0 deletions packages/libs/error-stack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ anyhow = { version = "1.0.65", default-features = false, optional = true }
eyre = { version = "0.6", default-features = false, optional = true }
owo-colors = { version = "3", default-features = false, optional = true, features = ['supports-colors'] }
serde = { version = "1", default-features = false, optional = true }
spin = { version = "0.9.4", default-features = false, optional = true, features = ['rwlock', 'once'] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does 0.9 also work for our use cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requirement relaxed in 122a119

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!


[dev-dependencies]
serde = { version = "1.0.148", features = ["derive"] }
Expand All @@ -47,6 +48,7 @@ spantrace = ["dep:tracing-error", "std"]
std = ["anyhow?/std"]
eyre = ["dep:eyre", "std"]
serde = ["dep:serde"]
hooks = ['dep:spin']

[package.metadata.docs.rs]
all-features = true
Expand Down
20 changes: 12 additions & 8 deletions packages/libs/error-stack/src/fmt/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,14 +720,14 @@ fn into_boxed_hook<T: Send + Sync + 'static>(
/// [`Display`]: core::fmt::Display
/// [`Debug`]: core::fmt::Debug
/// [`.insert()`]: Hooks::insert
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) struct Hooks {
// We use `Vec`, instead of `HashMap` or `BTreeMap`, so that ordering is consistent with the
// insertion order of types.
pub(crate) inner: Vec<(TypeId, BoxedHook)>,
}

#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
impl Hooks {
pub(crate) fn insert<T: Send + Sync + 'static>(
&mut self,
Expand Down Expand Up @@ -755,20 +755,21 @@ impl Hooks {
mod default {
#![allow(unused_imports)]

#[cfg(any(rust_1_65, feature = "spantrace"))]
use alloc::format;
use alloc::{vec, vec::Vec};
use alloc::{format, vec, vec::Vec};
use core::{
any::TypeId,
panic::Location,
sync::atomic::{AtomicBool, Ordering},
};
#[cfg(rust_1_65)]
#[cfg(all(rust_1_65, feature = "std"))]
use std::backtrace::Backtrace;
#[cfg(feature = "std")]
use std::sync::Once;

#[cfg(feature = "pretty-print")]
use owo_colors::{OwoColorize, Stream};
#[cfg(all(not(feature = "std"), feature = "hooks"))]
use spin::once::Once;
#[cfg(feature = "spantrace")]
use tracing_error::SpanTrace;

Expand All @@ -787,6 +788,9 @@ mod default {
//
// > If the given closure recursively invokes call_once on the same Once instance the exact
// > behavior is not specified, allowed outcomes are a panic or a deadlock.
//
// This limitation is not present for the implementation from the spin crate, but for
// simplicity and readability the extra guard is kept.
static INSTALL_BUILTIN_RUNNING: AtomicBool = AtomicBool::new(false);

// This has minimal overhead, as `Once::call_once` calls `.is_completed` as the short path
Expand All @@ -800,7 +804,7 @@ mod default {

Report::install_debug_hook::<Location>(location);

#[cfg(rust_1_65)]
#[cfg(all(feature = "std", rust_1_65))]
Report::install_debug_hook::<Backtrace>(backtrace);

#[cfg(feature = "spantrace")]
Expand All @@ -819,7 +823,7 @@ mod default {
context.push_body(format!("at {location}"));
}

#[cfg(rust_1_65)]
#[cfg(all(feature = "std", rust_1_65))]
fn backtrace(backtrace: &Backtrace, context: &mut HookContext<Backtrace>) {
let idx = context.increment_counter();

Expand Down
43 changes: 23 additions & 20 deletions packages/libs/error-stack/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
//! [`atomic`]: std::sync::atomic
//! [`Error::provide`]: core::error::Error::provide

#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
mod hook;

use alloc::{
Expand All @@ -159,9 +159,9 @@ use core::{
mem,
};

#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
pub use hook::HookContext;
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) use hook::{install_builtin_hooks, Hooks};
#[cfg(feature = "pretty-print")]
use owo_colors::{OwoColorize, Stream, Style as OwOStyle};
Expand Down Expand Up @@ -670,37 +670,37 @@ impl Opaque {

fn debug_attachments_invoke<'a>(
frames: impl IntoIterator<Item = &'a Frame>,
#[cfg(feature = "std")] context: &mut HookContext<Frame>,
#[cfg(any(feature = "std", feature = "hooks"))] context: &mut HookContext<Frame>,
) -> (Opaque, Vec<String>) {
let mut opaque = Opaque::new();

let body = frames
.into_iter()
.map(|frame| match frame.kind() {
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
FrameKind::Context(_) => Some(
Report::invoke_debug_format_hook(|hooks| hooks.call(frame, context))
.then(|| context.take_body())
.unwrap_or_default(),
),
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
FrameKind::Attachment(AttachmentKind::Printable(attachment)) => Some(
Report::invoke_debug_format_hook(|hooks| hooks.call(frame, context))
.then(|| context.take_body())
.unwrap_or_else(|| vec![attachment.to_string()]),
),
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
FrameKind::Attachment(AttachmentKind::Opaque(_)) => {
Report::invoke_debug_format_hook(|hooks| hooks.call(frame, context))
.then(|| context.take_body())
}
#[cfg(not(feature = "std"))]
#[cfg(not(any(feature = "std", feature = "hooks")))]
FrameKind::Context(_) => Some(vec![]),
#[cfg(not(feature = "std"))]
#[cfg(not(any(feature = "std", feature = "hooks")))]
FrameKind::Attachment(AttachmentKind::Printable(attachment)) => {
Some(vec![attachment.to_string()])
}
#[cfg(all(not(feature = "std"), feature = "pretty-print"))]
#[cfg(all(not(any(feature = "std", feature = "hooks")), feature = "pretty-print"))]
FrameKind::Attachment(AttachmentKind::Opaque(_)) => frame
.downcast_ref::<core::panic::Location<'static>>()
.map(|location| {
Expand All @@ -710,7 +710,10 @@ fn debug_attachments_invoke<'a>(
.to_string(),
]
}),
#[cfg(all(not(feature = "std"), not(feature = "pretty-print")))]
#[cfg(all(
not(any(feature = "std", feature = "hooks")),
not(feature = "pretty-print")
))]
FrameKind::Attachment(AttachmentKind::Opaque(_)) => frame
.downcast_ref::<core::panic::Location>()
.map(|location| vec![format!("at {location}")]),
Expand All @@ -731,13 +734,13 @@ fn debug_attachments_invoke<'a>(
fn debug_attachments<'a>(
position: Position,
frames: impl IntoIterator<Item = &'a Frame>,
#[cfg(feature = "std")] context: &mut HookContext<Frame>,
#[cfg(any(feature = "std", feature = "hooks"))] context: &mut HookContext<Frame>,
) -> Lines {
let last = matches!(position, Position::Final);

let (opaque, entries) = debug_attachments_invoke(
frames,
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
context,
);
let opaque = opaque.render();
Expand Down Expand Up @@ -863,7 +866,7 @@ fn debug_render(head: Lines, contexts: VecDeque<Lines>, sources: Vec<Lines>) ->
fn debug_frame(
root: &Frame,
prefix: &[&Frame],
#[cfg(feature = "std")] context: &mut HookContext<Frame>,
#[cfg(any(feature = "std", feature = "hooks"))] context: &mut HookContext<Frame>,
) -> Vec<Lines> {
let (stack, sources) = collect(root, prefix);
let (stack, prefix) = partition(&stack);
Expand Down Expand Up @@ -904,7 +907,7 @@ fn debug_frame(
Position::Inner
},
once(head).chain(body),
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
context,
);
head_context.then(body)
Expand All @@ -920,7 +923,7 @@ fn debug_frame(
debug_frame(
source,
&prefix,
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
context,
)
},
Expand All @@ -943,18 +946,18 @@ fn debug_frame(

impl<C> Debug for Report<C> {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
let mut context = HookContext::new(fmt.alternate());

#[cfg_attr(not(feature = "std"), allow(unused_mut))]
#[cfg_attr(not(any(feature = "std", feature = "hooks")), allow(unused_mut))]
let mut lines = self
.current_frames()
.iter()
.flat_map(|frame| {
debug_frame(
frame,
&[],
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
&mut context,
)
})
Expand All @@ -975,7 +978,7 @@ impl<C> Debug for Report<C> {
.collect::<Vec<_>>()
.join("\n");

#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
{
let appendix = context
.appendix()
Expand Down
26 changes: 23 additions & 3 deletions packages/libs/error-stack/src/hook.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
use std::sync::RwLock;
use alloc::vec::Vec;

use crate::{
fmt::{install_builtin_hooks, HookContext, Hooks},
Report,
};

#[cfg(feature = "std")]
type RwLock<T> = std::sync::RwLock<T>;

// Generally the std mutex is faster than spin, so if both `std` and `hooks` is enabled we use the
// std variant.
#[cfg(all(not(feature = "std"), feature = "hooks"))]
type RwLock<T> = spin::rwlock::RwLock<T>;

static FMT_HOOK: RwLock<Hooks> = RwLock::new(Hooks { inner: Vec::new() });

impl Report<()> {
Expand Down Expand Up @@ -139,24 +147,36 @@ impl Report<()> {
/// </pre>
///
/// [`Error::provide`]: std::error::Error::provide
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
pub fn install_debug_hook<T: Send + Sync + 'static>(
hook: impl Fn(&T, &mut HookContext<T>) + Send + Sync + 'static,
) {
install_builtin_hooks();

#[cfg(feature = "std")]
let mut lock = FMT_HOOK.write().expect("should not be poisoned");

// The spin RwLock cannot panic
#[cfg(all(not(feature = "std"), feature = "hooks"))]
let mut lock = FMT_HOOK.write();

lock.insert(hook);
}

/// Returns the hook that was previously set by [`install_debug_hook`]
///
/// [`install_debug_hook`]: Self::install_debug_hook
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) fn invoke_debug_format_hook<T>(closure: impl FnOnce(&Hooks) -> T) -> T {
install_builtin_hooks();

#[cfg(feature = "std")]
let hook = FMT_HOOK.read().expect("should not be poisoned");

// The spin RwLock cannot panic
#[cfg(all(not(feature = "std"), feature = "hooks"))]
let hook = FMT_HOOK.read();

closure(&hook)
}
}
9 changes: 5 additions & 4 deletions packages/libs/error-stack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,10 @@
//!
//! Feature | Description | default
//! ---------------|--------------------------------------------------------------------|----------
//! `std` | Enables support for [`Error`] and, on nightly, [`Backtrace`] | enabled
//! `std` | Enables support for [`Error`], and, on Rust 1.65+, [`Backtrace`] | enabled
//! `pretty-print` | Provide color[^color] and use of unicode in [`Debug`] output | enabled
//! `spantrace` | Enables automatic capturing of [`SpanTrace`]s | disabled
//! `hooks` | Enables hooks on `no-std` platforms using spin locks | disabled
//! `anyhow` | Provides `into_report` to convert [`anyhow::Error`] to [`Report`] | disabled
//! `eyre` | Provides `into_report` to convert [`eyre::Report`] to [`Report`] | disabled
//!
Expand Down Expand Up @@ -475,11 +476,11 @@ mod report;
mod result;

mod context;
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
pub mod fmt;
#[cfg(not(feature = "std"))]
#[cfg(not(any(feature = "std", feature = "hooks")))]
mod fmt;
#[cfg(feature = "std")]
#[cfg(any(feature = "std", feature = "hooks"))]
mod hook;
#[cfg(feature = "serde")]
mod serde;
Expand Down