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
Currently, client.eventFromException only calls the Cause method on the exception once.
Using github.com/pkg/errors, if we use the errors.Wrap function to wrap our error (let's say of type TestError), this gives us the following chain: errors.WithStack -> errors.WithMessage -> TestError
and since the Cause method returns the direct cause, not the underlying one, we end up with type errors.WithMessage being reported to Sentry. (Instead of TestError.)
We can do better than github.com/pingcap/errors.Cause and github.com/pkg/errors.Cause because instead of reporting a single error we can report all errors in the chain.
Pending a clarification on the Sentry API for how best to implement the last point. There are at least two possibilities:
Currently,
client.eventFromException
only calls theCause
method on the exception once.Using
github.com/pkg/errors
, if we use theerrors.Wrap
function to wrap our error (let's say of typeTestError
), this gives us the following chain:errors.WithStack
->errors.WithMessage
->TestError
and since the Cause method returns the direct cause, not the underlying one, we end up with type
errors.WithMessage
being reported to Sentry. (Instead ofTestError
.)I believe the underlying cause should be reported to sentry, i.e.
eventFromException
should determine cause in the same way github.com/pingcap/errors and github.com/pkg/errors do (both call the Cause method repeatedly, the first recursively, the second iteratively).The proposed change of this part:
Let me know what you think and whether I can open a PR to fix this :)
The text was updated successfully, but these errors were encountered: