Further error-logging improvements #333
Conversation
|
@akshayjshah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @prashantv, @pravj and @creiht to be potential reviewers. |
akshayjshah
force-pushed
the
ajs-errors
branch
from
4a5cbda to
4a27f01
Compare
array.go
Outdated
| // Re-use the error field's logic, which supports non-standard error types. | ||
| Error(e.error).AddTo(enc) | ||
| return nil | ||
|
|
| { | ||
| "errors", | ||
| Errors("", []error{nil, errors.New("foo"), nil, errors.New("bar")}), | ||
| []interface{}{map[string]interface{}{"error": "foo"}, map[string]interface{}{"error": "bar"}}, |
There was a problem hiding this comment.
should we add a test which also adds errorVerbose?
There was a problem hiding this comment.
Sure, I can. Those code paths are already covered, but doesn't hurt to do it again.
| error | ||
| } | ||
|
|
||
| func (e *errArrayElem) MarshalLogObject(enc zapcore.ObjectEncoder) error { |
There was a problem hiding this comment.
this does mean our output for []error ends up being a list of objects which have a key error. I don't have strong opinions on this, but it does seem a little strange.
There was a problem hiding this comment.
Yeah, it's definitely a little odd. I'm honestly not sure what else to do - it seems worse to have arrays that mix strings and objects. Would you prefer that? Or separate arrays of the errors and verbose errors?
First, add support for customizing error fields' keys; now that we support rich errors, we can't simply rely on string fields.
Second, use that support in
AnyandErrors. This should complete our support forgithub.aaakk.us.kg/pkg/errors.Fully resolves #303.