You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Our project uses quite a bit of panic()ing for flow control in http middlewares. Those middlewares functions try to recover from a panic, but only if it a panic they can handle (example: a db/tx middleware only handles panics that look like a db concurrency issues).
If the error cannot be handled, the recovered value ispaniced again -- which in practice works quite well. Go simply adds any extra panic recovery to the stack, so the first panic point can be found if you just look far enough down.
An example of a repanic raw stack trace submitted to bugsnag:
The underlying problem here is app/conv.go:151 where a nil pointer is deferenced. However, because sqli/tx.go re-panics, the error is currently grouped under sqli/tx.go:74 (runtime/ stuff is outside ProjectPackages).
I think in almost all cases the first (lowest) panic in the stack would be of most interest, so would expect the grouping logic to scan down the stack and look for the oldest panic, and then find the first function in a ProjectPackage below it.
The text was updated successfully, but these errors were encountered:
EmielM
changed the title
Support proper grouping for "re-panics"
Support proper grouping for repanics
Jun 20, 2017
Ah yes, I'm aware I could probably bugsnag-go.errors.Wrap() the recovered values at all the sites I'm repanicing them, but that feels like quite the buy-in to your custom errors package while the actual error site can be extracted from the stack in the final recovery.
I apologize for the delay here, @EmielM. This is a great idea, and probably something we could do by default. On other platforms, we already handle nested causes automatically, and could probably do the same for Go, separating each instance of gopanic. Your workaround looks like a good place to start for some test cases. I'm moving this into our roadmap as there are a few improvements we can probably make here for go stacktraces.
Our project uses quite a bit of
panic()
ing for flow control in http middlewares. Those middlewares functions try to recover from a panic, but only if it a panic they can handle (example: a db/tx middleware only handles panics that look like a db concurrency issues).If the error cannot be handled, the recovered value is
panic
ed again -- which in practice works quite well. Go simply adds any extra panic recovery to the stack, so the first panic point can be found if you just look far enough down.An example of a re
panic
raw stack trace submitted to bugsnag:The underlying problem here is
app/conv.go:151
where a nil pointer is deferenced. However, becausesqli/tx.go
re-panic
s, the error is currently grouped undersqli/tx.go:74
(runtime/
stuff is outsideProjectPackages
).I think in almost all cases the first (lowest) panic in the stack would be of most interest, so would expect the grouping logic to scan down the stack and look for the oldest panic, and then find the first function in a
ProjectPackage
below it.The text was updated successfully, but these errors were encountered: