assertion failed: meta.is_event()
dinhani opened this issue · 3 comments
What crate(s) in this repo are involved in the problem?
console-api
What is the issue?
console-api
is panicking when running in debug mode in this debug_assert!
No idea if this is expected or not, but I identified that an event of kind HINT
is reaching that point and triggering the assertion to fail.
It comes from the external crate jsonrpsee-core
, so I cannot easily change it.
Metadata {
name: "enabled /home/renato/.cargo/registry/src/index.crates.io-6f17d22bba15001f/jsonrpsee-core-0.22.5/src/tracing.rs:13",
target: "jsonrpsee_core::tracing::client",
level: Level(Trace),
module_path: "jsonrpsee_core::tracing::client",
location: /home/renato/.cargo/registry/src/index.crates.io-6f17d22bba15001f/jsonrpsee-core-0.22.5/src/tracing.rs:13,
fields: {},
callsite: Identifier(0x559831eadc48),
kind: Kind(HINT)
}
How can the bug be reproduced?
let client = HttpClientBuilder::default().build(jsonrpc_url).unwrap();
let _ = client.request::<bool, Vec<()>>("net_listening", vec![]).await
Logs, error output, etc
thread 'tokio' panicked at /home/renato/projects/console/console-api/src/common.rs:50:13:
assertion failed: meta.is_event()
Versions
├── console-subscriber v0.2.0
│ ├── console-api v0.6.0
Possible solution
Maybe filtering out invalid metadata kinds before calling the function or using TryFrom
instead.
The function is being called from aggregator.rs in console-subscriber
.
Additional context
No response
Would you like to work on fixing this bug?
yes
@dinhani Thanks for your issue report!
I would say that this is definitely incorrect on console-api
side, even in debug mode we should be more resilient.
This will only happen if the console-subscriber
is receiving traces that aren't from Tokio, since in the Tokio instrumentation we only use events and spans, but if we're going to have this assertion in place, we should be filtering out non-event/non-span callsites and not sending them on to the aggregator (we won't do anything with them anyway).
If you're interested in working on this, I propose that we check that the metadata received by register_callsite
is either an event or a span, and otherwise we return Interest::never()
and don't do any more:
console/console-subscriber/src/lib.rs
Line 531 in a0d20fd
We could add a test for this behaviour, although it looks like our test set-up doesn't fail the test when the subscriber thread panics, which is weird. I'll look at that separately.
I tested my project with the suggested change at register_callsite
and it worked. There are no more panics.
I sent the suggested PR, but I still need to check how to test it properly in this project.