-
Notifications
You must be signed in to change notification settings - Fork 13
Debug ergonomics #280
Description
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:
- 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) - If we change EDHOCError to be a
struct { EDHOCErrorVariant, Option<'static str> }, we'll have to construct them through the internal variant asEDHOCErrorVariant::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. - 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?