Capture backtraces when codegen goes awry #128
Conversation
There was a problem hiding this comment.
Happy to merge this as an improvement, but I think longer term we might want to move away from syn::Error, and create our own error type that generates a backtrace when it is created; this would have an impl of From<syn::Error>, and in that impl we would grab the backtrace, which would then work anywhere there is ?.
| // This is the one step where we can't readily intercept the error with logged_syn_error | ||
| let mut items: Items = syn::parse_str(code_str).map_err(|e| { | ||
| debug!("{}", Backtrace::capture()); | ||
| e | ||
| })?; |
There was a problem hiding this comment.
So this is only going to capture a backtrace at this point, which might not be what we want? In particular it won't tell us anything about what where in our parse impls we failed.
There was a problem hiding this comment.
If we fail parsing entirely, e.g. I go add DUCK to some_input.rs, then we hit this w/o hitting any of our parse stuff. if I deliberately break something, say change u16 to u16z somewhere, we get a trace to the right place in our code.
| } | ||
| } | ||
|
|
||
| pub(crate) fn logged_syn_error<T: Display>(span: Span, message: T) -> syn::Error { |
There was a problem hiding this comment.
So this is an improvement, but as we're only doing this with errors that we create explicitly (as opposed to errors that occur during another parse method, for instance when we do let name = input.parse::<syn::Ident>()?).
And unfortunately, errors of the other type (generated in syn, and merely propagated by us) are probably the majority of our errors?
There was a problem hiding this comment.
When I tried introducing some basic errors, e.g. a type that didn't exist, I did get a trace. In any case, IIUC using our own error that auto-captures traces should entirely resolve this.
That does sound much nicer. EDIT: I tried this and it seemed to work out worse because in This does leave us with the problem that we miss backtracing places where we use ?, I'm not sure wha tthe best way to tackle that is. I think I'll merge as-is for now, but without closing the capture backtrace issue, as what's here does make life easier despite being imperfect. |
Feel free to merge if acceptable. Helps with #82, though spots where we propagate an error with ? still get missed. Baby steps.