search

guregu / null

reasonable handling of nullable values
BSD 2-Clause "Simplified" License
1.84k stars 238 forks source link

Not to use fmt.Errorf for UnmarshalJSON error #66

Closed DhZero closed 3 years ago

DhZero commented 3 years ago

What version of Go are you using (go version)?

$ go version
go version go1.15.6 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

https://play.golang.org/p/7kgBsW80Mzf

What is the problem?

If we would have not empty decoder.errorContext.FieldStack and we received error of fmt.wrapError type from UnmarshalJSON then later addErrorContext wouldn't be able to set proper value for err.Field and err.Struct because error type won't match with *UnmarshalTypeError.

https://github.com/golang/go/blob/a7e16abb22f1b249d2691b32a5d20206282898f2/src/encoding/json/decode.go#L181-L183 https://github.com/golang/go/blob/a7e16abb22f1b249d2691b32a5d20206282898f2/src/encoding/json/decode.go#L255-L258

guregu commented 3 years ago

Use errors.As Edit: nevermind, misunderstood question.

DhZero commented 3 years ago

Thanks for fast replay, but how it can help to solve the issue? Yeah, you can work with error like that after unmarshaling is over but content of error is not going to be fixed. And I can't make changes in encoding/json package itself.

Maybe I didn't understand your idea, then could you explain it.

guregu commented 3 years ago

Oh, my bad! I misunderstood your question, sorry. Thanks for the example code, I see where you're coming from. Let me see if there's anything that we can do.

guregu commented 3 years ago

OK I understand it now.

We have these options:

The API for null.v3 and null.v4 is basically the same, so switching to that is the easiest solution. TBH I think this should count as a bug in the Go standard library, so I think a patch is the cleanest solution.

DhZero commented 3 years ago

Thanks, I will check out v3.

Also I think to deal with it as a bug for the Go standard library might be the best solution, because not only your package/struct can wrap errors.

guregu commented 3 years ago

Thanks for making the issue. Going to link it here to keep track: https://github.com/golang/go/issues/45449