Update error reporting analytics #2489
Conversation
MDrakos
commented
Apr 13, 2023
•
MDrakos
commented
 DX-495 As a tech-lead I can diagnose the failure reasons of commands
|
pkg/cmdlets/errors/errors.go
Outdated
| } | ||
|
|
||
| logging.Debug("Reporting error:\n%s\nCreated at:\n%s", errs.Join(err, "\n").Error(), stack) | ||
| an.EventWithLabel(anaConst.CatDebug, action, strings.Join(label, " "), &dimensions.Values{ | ||
| Error: p.StrP(err.Error()), |
There was a problem hiding this comment.
If action == anaConst.ActCommandInputError this needs to be the topmost input error, rather than the topmost error (the two may not be the same).
pkg/cmdlets/errors/errors.go
Outdated
| for err != nil { | ||
| if locale.IsInputErrorNonRecursive(err) { | ||
| errorMsg = locale.ErrorMessage(err) | ||
| break | ||
| } | ||
| err = errors.Unwrap(err) | ||
| } |
There was a problem hiding this comment.
You forgot to update errorMsg.
Also, you can probably simplify this by using the first slice entry returned by locale.UnwrapError().
There was a problem hiding this comment.
Do we want to update the error to a special static message if we exit the for loop without encountering an input error? errorMsg is set on line 149 and updated when we encounter the input error.
The first entry in locale.UnwrapError() is not guaranteed to be an input error, just a localized one.
There was a problem hiding this comment.
Ah I forgot about that distinction!
Looking at it again, I think your logic is sound! Makes sense to use the non-input error if we didn't encounter one (which shouldn't happen if the input error functions work as intended).
pkg/cmdlets/errors/errors.go
Outdated
| for err != nil { | ||
| if locale.IsInputErrorNonRecursive(err) { | ||
| errorMsg = locale.ErrorMessage(err) | ||
| break | ||
| } | ||
| err = errors.Unwrap(err) | ||
| } |
There was a problem hiding this comment.
Ah I forgot about that distinction!
Looking at it again, I think your logic is sound! Makes sense to use the non-input error if we didn't encounter one (which shouldn't happen if the input error functions work as intended).
