-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Return a key with the error on Value.Populate failures #427
Conversation
cf514df
to
40fde52
Compare
config/decoder.go
Outdated
@@ -206,6 +207,10 @@ func addSeparator(key string) string { | |||
return key | |||
} | |||
|
|||
func addKeyToError(key string, err error) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: errorWithKey(err error, key string) is more meaningful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, make sense
config/decoder.go
Outdated
@@ -342,7 +347,8 @@ func (d *decoder) mapping(childKey string, value reflect.Value, def string) erro | |||
for key := range v { | |||
subKey := fmt.Sprintf("%v", key) | |||
if subKey == "" { | |||
return fmt.Errorf("empty key leads to ambiguity for path: %q", childKey) | |||
// We can confuse an empty map key with a root element. | |||
return addKeyToError(childKey, errors.New("empty map key is ambigious")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is it worth adding another function, newErrorWithKey(msg string, key string), so we don't have to create error twice? A small performance win but not sure how much of a difference it makes. Will leave this to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather to keep an error here, so stacktrace is not going to get too long.
We provide only information about types, omitting the offending key, which can be confusing for users. The easiest way would be to return error from decoder.unmarshal, but because it is recursive, we need to track leafs and attach the offending key on leafs.
We provide only information about types, omitting the offending key, which can be confusing for users. The easiest way would be to return error from
decoder.unmarshal
, but because it is recursive, we need to track leafs and attach the offending key on leafs.