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

GEN-35: error-stack implement serde hooks #1558

Draft
wants to merge 55 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
2885f9b
feat: convert `serde` to module
indietyp Nov 30, 2022
fb4520b
feat: start serialize hook
indietyp Nov 30, 2022
50d9993
feat: continue work on hooks
indietyp Dec 6, 2022
f6b721e
feat: serde static and dynamic hooks
indietyp Dec 17, 2022
6b82a51
feat: serde lifetime fun
indietyp Dec 17, 2022
a3167a1
fix: compile errors
indietyp Dec 17, 2022
b755a0c
Merge branch 'main' into bm/es/serde-hooks
indietyp Dec 21, 2022
5725e59
feat: dispatch closure through fn
indietyp Dec 21, 2022
724a0a0
feat: use traits to constraint lifetime of `U`
indietyp Dec 21, 2022
3a6f360
feat: use traits to constraint lifetime of `U`
indietyp Dec 21, 2022
5c44c7d
feat: it works?
indietyp Dec 22, 2022
4ae6bd5
feat: install custom serde hook
indietyp Dec 22, 2022
e321ff5
feat: integration test
indietyp Dec 22, 2022
3b85638
feat: integration test
indietyp Dec 22, 2022
9f5d32b
feat: introduce hooks into serializer
indietyp Dec 22, 2022
bdea88b
fix: WIP
indietyp Dec 22, 2022
4ab9d0b
fix: reference shenanigans
indietyp Dec 22, 2022
ddac2d2
fix: more references!
indietyp Dec 22, 2022
05baaaf
feat: cfg hook capabilities
indietyp Dec 22, 2022
1c4871b
fix: lint
indietyp Dec 22, 2022
1c05422
test: custom hook snapshot
indietyp Dec 22, 2022
77f2008
feat: built-in hooks
indietyp Dec 23, 2022
91ced98
feat: unify helpers of `test_debug` and `test_serialize`
indietyp Dec 23, 2022
845fff8
fix: compilation errors
indietyp Dec 23, 2022
0abe3db
chore: update snapshots
indietyp Dec 23, 2022
1d7221d
fix: lint
indietyp Dec 23, 2022
38ffd86
chore: update snapshots
indietyp Dec 23, 2022
ceab0d2
Merge branch 'main' into bm/es/serde-hooks
indietyp Dec 23, 2022
1d976d0
chore: snapshots are now json
indietyp Dec 23, 2022
aaca87f
Merge branch 'bm/es/serde-hooks' of github.com:hashintel/hash into bm…
indietyp Dec 23, 2022
b37c9a4
fix: import visibility for `Serde`
indietyp Dec 23, 2022
81253a2
feat: nightly only test for closures
indietyp Dec 24, 2022
43c600b
chore: remove incorrect comment
indietyp Dec 24, 2022
83edbf2
feat: `serde::Hooks`-> `SerdeHooks`, `fmt::Hooks` -> `FmtHooks`
indietyp Jan 2, 2023
0941b84
feat: `DynamicFn` -> `SerializeFn`
indietyp Jan 2, 2023
7b39856
chore: update snapshots
indietyp Jan 2, 2023
efc3db4
feat: backtrace builtin hook
indietyp Jan 2, 2023
3f57310
Merge branch 'main' into bm/es/serde-hooks
indietyp Jan 2, 2023
8c30be3
chore: remove unneeded lifetime
indietyp Jan 2, 2023
35fdfe2
chore: for now allow missing docs on undocumented methods
indietyp Jan 2, 2023
28780d3
Merge branch 'bm/es/serde-hooks' of github.com:hashintel/hash into bm…
indietyp Jan 2, 2023
2ccccfe
fix: lint
indietyp Jan 3, 2023
f4c30af
Merge remote-tracking branch 'origin/main' into bm/es/serde-hooks
indietyp Jan 11, 2023
491c94b
fix: merge conflicts
indietyp Jan 11, 2023
c528da9
chore: update snapshots
indietyp Jan 11, 2023
b44832e
test: remove backtrace for serialize tests
indietyp Jan 11, 2023
cb766d0
feat: disable backtrace for serde
indietyp Jan 12, 2023
47364dd
chore: update stable snaps
indietyp Jan 12, 2023
14506e0
Merge remote-tracking branch 'origin/main' into bm/es/serde-hooks
indietyp Feb 13, 2023
8a7b3fe
fix: merge conflicts
indietyp Feb 13, 2023
1b4db94
fix: merge conflicts (II)
indietyp Feb 13, 2023
cce1439
fix: merge conflicts (III)
indietyp Feb 13, 2023
58b90e8
fix: merge conflicts (IV)
indietyp Feb 13, 2023
64077b4
feat: start with serde config
indietyp Feb 13, 2023
d9b1f78
feat: start with serde config
indietyp Feb 13, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 4 additions & 7 deletions libs/error-stack/Cargo.toml
Expand Up @@ -18,10 +18,12 @@ keywords = ["error", "errorstack", "error-handling", "report", "no_std"]
categories = ["rust-patterns", "no-std"]

[dependencies]
tracing-core = { version = "0.1", optional = true, default_features = false }
tracing-error = { version = "0.2", optional = true, default_features = false }
anyhow = { version = "1.0.65", default-features = false, optional = true }
eyre = { version = "0.6", default-features = false, optional = true }
serde = { version = "1", default-features = false, optional = true }
erased-serde = { version = "0.3.23", default-features = false, optional = true, features = ['alloc'] }
spin = { version = "0.9", default-features = false, optional = true, features = ['rwlock', 'once'] }

[dev-dependencies]
Expand All @@ -46,10 +48,10 @@ rustc_version = "0.4"
[features]
default = ["std"]

spantrace = ["dep:tracing-error", "std"]
spantrace = ["dep:tracing-error", "dep:tracing-core", "std"]
std = ["anyhow?/std"]
eyre = ["dep:eyre", "std"]
serde = ["dep:serde"]
serde = ["dep:serde", "dep:erased-serde"]
hooks = ['dep:spin']

[package.metadata.docs.rs]
Expand All @@ -72,8 +74,3 @@ required-features = ["std"]
[[example]]
name = "detect"
required-features = ['std']


[[test]]
name = "common"
test = false
4 changes: 2 additions & 2 deletions libs/error-stack/Makefile.toml
Expand Up @@ -7,9 +7,9 @@ CARGO_TEST_HACK_FLAGS = { source = "${GITHUB_EVENT_NAME}", default_value = "${CO
CARGO_INSTA_TEST_FLAGS = "--no-fail-fast"
CARGO_RUSTDOC_HACK_FLAGS = ""
CARGO_DOC_HACK_FLAGS = ""
# The only features that are of relevance are: spantrace, std, hooks
# The only features that are of relevance are: spantrace, pretty-print, std, hooks, serde
# all others do not change the output
CARGO_INSTA_TEST_HACK_FLAGS = "--keep-going --feature-powerset --include-features spantrace,std,hooks"
CARGO_INSTA_TEST_HACK_FLAGS = "--keep-going --feature-powerset --include-features spantrace,std,hooks,serde"

# Workaround for https://github.com/taiki-e/cargo-hack/issues/161
# instead, we should pass `--ignore-unknown-features` to `cargo hack`
Expand Down
8 changes: 4 additions & 4 deletions libs/error-stack/src/fmt.rs
Expand Up @@ -144,7 +144,7 @@

mod charset;
mod color;
mod config;
pub(crate) mod config;
#[cfg(any(feature = "std", feature = "hooks"))]
mod hook;
mod location;
Expand All @@ -169,7 +169,7 @@ pub use color::ColorMode;
#[cfg(any(feature = "std", feature = "hooks"))]
pub use hook::HookContext;
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) use hook::{install_builtin_hooks, Format, Hooks};
pub(crate) use hook::{install_builtin_debug_hooks, FmtHooks, Format};
#[cfg(not(any(feature = "std", feature = "hooks")))]
use location::LocationDisplay;

Expand Down Expand Up @@ -649,7 +649,7 @@ fn debug_context(context: &dyn Context, mode: ColorMode) -> Lines {
.collect()
}

struct Opaque(usize);
pub(crate) struct Opaque(usize);

impl Opaque {
const fn new() -> Self {
Expand All @@ -675,7 +675,7 @@ impl Opaque {
}
}

fn debug_attachments_invoke<'a>(
pub(crate) fn debug_attachments_invoke<'a>(
frames: impl IntoIterator<Item = &'a Frame>,
config: &mut Config,
) -> (Opaque, Vec<String>) {
Expand Down
17 changes: 10 additions & 7 deletions libs/error-stack/src/fmt/hook.rs
Expand Up @@ -6,9 +6,12 @@
use alloc::{boxed::Box, string::String, vec::Vec};
use core::{any::TypeId, mem};

pub(crate) use default::install_builtin_hooks;
pub(crate) use default::install_builtin_debug_hooks;

use crate::fmt::{charset::Charset, ColorMode, Frame};
use crate::{
fmt::{charset::Charset, ColorMode, Frame},
hook::context::impl_hook_context,
};

pub(crate) struct Format {
alternate: bool,
Expand Down Expand Up @@ -42,7 +45,7 @@ impl Format {
}
}

crate::hook::context::impl_hook_context! {
impl_hook_context! {
/// Carrier for contextual information used across hook invocations.
///
/// `HookContext` has two fundamental use-cases:
Expand Down Expand Up @@ -418,14 +421,14 @@ fn into_boxed_hook<T: Send + Sync + 'static>(
/// [`SpanTrace`]: tracing_error::SpanTrace
/// [`Display`]: core::fmt::Display
/// [`Debug`]: core::fmt::Debug
/// [`.insert()`]: Hooks::insert
pub(crate) struct Hooks {
/// [`.insert()`]: FmtHooks::insert
pub(crate) struct FmtHooks {
// 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)>,
}

impl Hooks {
impl FmtHooks {
pub(crate) fn insert<T: Send + Sync + 'static>(
&mut self,
hook: impl Fn(&T, &mut HookContext<T>) + Send + Sync + 'static,
Expand Down Expand Up @@ -472,7 +475,7 @@ mod default {
Report,
};

pub(crate) fn install_builtin_hooks() {
pub(crate) fn install_builtin_debug_hooks() {
// We could in theory remove this and replace it with a single AtomicBool.
static INSTALL_BUILTIN: Once = Once::new();

Expand Down
63 changes: 58 additions & 5 deletions libs/error-stack/src/hook.rs
Expand Up @@ -2,8 +2,10 @@ pub(crate) mod context;

use alloc::vec::Vec;

#[cfg(feature = "serde")]
use crate::serde::{install_builtin_serde_hooks, SerdeHooks, SerializeFn};
use crate::{
fmt::{install_builtin_hooks, Hooks},
fmt::{install_builtin_debug_hooks, FmtHooks},
Report,
};

Expand All @@ -15,7 +17,9 @@ type RwLock<T> = std::sync::RwLock<T>;
#[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() });
static FMT_HOOK: RwLock<FmtHooks> = RwLock::new(FmtHooks { inner: Vec::new() });
#[cfg(feature = "serde")]
static SERDE_HOOK: RwLock<SerdeHooks> = RwLock::new(SerdeHooks::new());

impl Report<()> {
/// Can be used to globally set a [`Debug`] format hook, for a specific type `T`.
Expand Down Expand Up @@ -153,7 +157,7 @@ impl Report<()> {
pub fn install_debug_hook<T: Send + Sync + 'static>(
hook: impl Fn(&T, &mut crate::fmt::HookContext<T>) + Send + Sync + 'static,
) {
install_builtin_hooks();
install_builtin_debug_hooks();

#[cfg(feature = "std")]
let mut lock = FMT_HOOK.write().expect("should not be poisoned");
Expand All @@ -169,8 +173,8 @@ impl Report<()> {
///
/// [`install_debug_hook`]: Self::install_debug_hook
#[cfg(any(feature = "std", feature = "hooks"))]
pub(crate) fn invoke_debug_format_hook<T>(closure: impl FnOnce(&Hooks) -> T) -> T {
install_builtin_hooks();
pub(crate) fn invoke_debug_format_hook<T>(closure: impl FnOnce(&FmtHooks) -> T) -> T {
install_builtin_debug_hooks();

#[cfg(feature = "std")]
let hook = FMT_HOOK.read().expect("should not be poisoned");
Expand All @@ -181,4 +185,53 @@ impl Report<()> {

closure(&hook)
}

// TODO: upcoming PR will add documentation
#[allow(missing_docs)]
#[cfg(all(any(feature = "std", feature = "hooks"), feature = "serde"))]
pub fn install_serde_hook<T: serde::Serialize + Send + Sync + 'static>() {
install_builtin_serde_hooks();

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

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

lock.insert_static::<T>();
}

// TODO: upcoming PR will add documentation
#[allow(missing_docs)]
#[cfg(all(any(feature = "std", feature = "hooks"), feature = "serde"))]
pub fn install_custom_serde_hook<T>(closure: impl for<'a> SerializeFn<'a, T>)
where
T: Send + Sync + 'static,
{
install_builtin_serde_hooks();

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

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

lock.insert_dynamic(closure);
}

#[cfg(all(any(feature = "std", feature = "hooks"), feature = "serde"))]
pub(crate) fn invoke_serde_hook<T>(closure: impl FnOnce(&SerdeHooks) -> T) -> T {
install_builtin_serde_hooks();

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

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

closure(&hook)
}
}
2 changes: 1 addition & 1 deletion libs/error-stack/src/hook/context.rs
Expand Up @@ -59,7 +59,7 @@ impl<T> Inner<T> {
}

macro_rules! impl_hook_context {
($(#[$meta:meta])* $vis:vis struct HookContext<$extra:ident> {..}) => {
($(#[$meta:meta])* $vis:vis struct HookContext<$extra:ty> { .. }) => {

// TODO: add link to serde hooks once implemented
// TODO: ideally we would want to make `HookContextInner` private, as it is an implementation
Expand Down
6 changes: 3 additions & 3 deletions libs/error-stack/src/lib.rs
Expand Up @@ -476,6 +476,8 @@ pub mod iter;

mod compat;
mod frame;
#[cfg(any(feature = "std", feature = "hooks"))]
mod hook;
mod macros;
mod report;
mod result;
Expand All @@ -484,10 +486,8 @@ mod context;
#[cfg(any(nightly, feature = "std"))]
mod error;
pub mod fmt;
#[cfg(any(feature = "std", feature = "hooks"))]
mod hook;
#[cfg(feature = "serde")]
mod serde;
pub mod serde;

pub use self::{
compat::IntoReportCompat,
Expand Down