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

Closed
wants to merge 55 commits into from
Closed
Show file tree
Hide file tree
Changes from 33 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions packages/libs/error-stack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ 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 }
owo-colors = { version = "3", default-features = false, optional = true, features = ['supports-colors'] }
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 @@ -31,7 +33,7 @@ futures = { version = "0.3.25", default-features = false, features = ["executor"
trybuild = "1.0.72"
tracing = "0.1.37"
tracing-subscriber = "0.3.16"
insta = { version = "1.22.0", features = ['filters', 'ron'] }
insta = { version = "1.22.0", features = ['filters', 'json'] }
regex = "1.7.0"
expect-test = "1.4.0"
ansi-to-html = "0.1.2"
Expand All @@ -44,10 +46,10 @@ rustc_version = "0.4"
default = ["std", "pretty-print"]

pretty-print = ["dep:owo-colors"]
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 @@ -66,8 +68,3 @@ required-features = ["std"]
[[example]]
name = "parse_config"
required-features = ["std"]


[[test]]
name = "common"
test = false
4 changes: 2 additions & 2 deletions packages/libs/error-stack/Makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ CARGO_TEST_HACK_FLAGS = "--workspace --optional-deps --feature-powerset --exclud
CARGO_INSTA_TEST_FLAGS = "--no-fail-fast"
CARGO_RUSTDOC_HACK_FLAGS = ""
CARGO_DOC_HACK_FLAGS = ""
# The only features that are of relevance are: spantrace, pretty-print, 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,pretty-print,std,hooks"
CARGO_INSTA_TEST_HACK_FLAGS = "--keep-going --feature-powerset --include-features spantrace,pretty-print,std,hooks,serde"


[tasks.test]
Expand Down
4 changes: 2 additions & 2 deletions packages/libs/error-stack/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ fn debug_context(context: &dyn Context) -> Lines {
.collect()
}

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

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

fn debug_attachments_invoke<'a>(
pub(crate) fn debug_attachments_invoke<'a>(
frames: impl IntoIterator<Item = &'a Frame>,
#[cfg(any(feature = "std", feature = "hooks"))] context: &mut HookContext<Frame>,
) -> (Opaque, Vec<String>) {
Expand Down
48 changes: 48 additions & 0 deletions packages/libs/error-stack/src/hook/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type RwLock<T> = std::sync::RwLock<T>;
type RwLock<T> = spin::rwlock::RwLock<T>;

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

Choose a reason for hiding this comment

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

We probably also fully qualify the path to Hooks here as well or import them as FmtHooks. Actually, as they are internal only, I'd prefer renaming both Hooks structs to FmtHooks and SerdeHooks

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 83edbf2

#[cfg(feature = "serde")]
static SERDE_HOOK: RwLock<crate::serde::Hooks> = RwLock::new(crate::serde::Hooks::new());

impl Report<()> {
/// Can be used to globally set a [`Debug`] format hook, for a specific type `T`.
Expand Down Expand Up @@ -181,4 +183,50 @@ impl Report<()> {

closure(&hook)
}

#[cfg(all(any(feature = "std", feature = "hooks"), feature = "serde"))]
pub fn install_serde_hook<T: serde::Serialize + Send + Sync + 'static>() {
crate::serde::install_builtin_hooks();
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, could we also fully qualify the debug hooks or rename the functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 83edbf2


#[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>();
}

#[cfg(all(any(feature = "std", feature = "hooks"), feature = "serde"))]
pub fn install_custom_serde_hook<F, T>(closure: F)
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use impl Trait syntax for F, so we can call install_custom_serde_hook::<MyType>(...)?

Copy link
Member Author

@indietyp indietyp Jan 2, 2023

Choose a reason for hiding this comment

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

fixed in 7b39856

where
F: for<'a> crate::serde::DynamicFn<'a, T>,
T: Send + Sync + 'static,
{
crate::serde::install_builtin_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(&crate::serde::Hooks) -> T) -> T {
crate::serde::install_builtin_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 packages/libs/error-stack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ mod fmt;
#[cfg(any(feature = "std", feature = "hooks"))]
mod hook;
#[cfg(feature = "serde")]
mod serde;
pub mod serde;

#[cfg(all(doc, any(feature = "std", feature = "hooks")))]
pub use hook::context::HookContext;
Expand Down
153 changes: 0 additions & 153 deletions packages/libs/error-stack/src/serde.rs

This file was deleted.

Loading