Debug ergonomics

[chrysn]

Currently, our errors are coarse-grained (which is fine), and are returned as results rather than panicking (which is also fine).

A consequence of this is that it is very hard to trace down where an error occurs, because you can't just set break points to the constructor of (say) an EDHOCError::ParsingError, so without a time-travelling debugger and without std printf, it is very hard to find where parsing actually failed. (For example, to find #279 (comment), I added a ParsingErrorS(&'static str) variant and changed all potential construction sites to include a label).

Thing is, I don't know a good way to do that so that we can have the labels around. The labels I'd like to have are not intended as content for the error messages, merely as debug output (winding up in the impl Debug of the errors), and on production builds, those should be elided at compile time. None of the options seem to work right:

1. If we add a ('static str) content to all the error variants, we'll have to add a label to all constructors (much work but OK), but we can't compile time disable it with a #[cfg()] unless we also cfg-gate the arguments in the constructors (tedious)
2. If we change EDHOCError to be a struct { EDHOCErrorVariant, Option<'static str> }, we'll have to construct them through the internal variant as EDHOCErrorVariant::ParsingError.with_label("foo"), need to add a .without_label() or .into() whenever we directly return errors (on ? paths we can skip it and use the automatic .into()), and the match branches grow more verbose.
3. We could simplify 2 a bit if the EDHOCError had associated constants EDHOCError::ParsingError = EDHOCError(EDHOCErrorVariant::ParsingError, None), but that requires duplicating the variants across both types, requires a clippy override to allow an associated constant to use enum variant naming conventions, and will be super confusing in match because you can list all the variants and then the compiler complains about unhandled variants.

So, the issue is a bit annoying (esp. during plug testing), but I like none of the solutions I can come up with. Do you have better ones?

[geonnave]

Yeah, normally I only do "hot debug", i.e. rust-gdb --tui target/debug/coapclient, but that needs to be interactive.

I think a simple and good enough solution would be a logging system, but of course we don't have stdout so, I also don't have a solution.

... or maybe we could have it just be enabled for native builds, and have it be a no-op for embedded ones; that would be of some help.
I don't recall though if it is easy to do, maybe something like #![cfg_attr(not(feature = "python-bindings"), no_std)] but for the logging definitions.

[chrysn]

Logging might be a good idea: We'll need to check whether pulling in the log crate is really as free as I think it is (when logging is not enabled), and whether hax successfully ignores it when disabled, but at worst we'd have a trace!() or error!() macro that by default does nothing but if enabled calls the log macros. Maybe we can find an easy idiom to wrap that around some errors at creation time.

[geonnave]

Btw, just found out about defmt (and defmt-or-log), might be useful.

[chrysn]

defmt is awesome. I was unaware of defmt-or-log, that even more so.

[geonnave]

After #283 and #295, would you consider this issue solved, @chrysn ?