proposal: encoding/json: wrap error from TextUnmarshaler/Unmarshaler to locate the problem in input

[Fazt01]

Issue:

Currently, unmarshaling json with json.Unmarshal() (or json.NewDecoder().Decode() ), when a value is being unmarshaled into a struct field that implements encoding.TextUnmarshaler (or the json.Unmarshaler) and the .UnmarshalText() returns an error, this error is returned unchanged from the whole json.Unmarshal function:

https://go.dev/play/p/G5ag-Cq2OMf

type CustomType string

func (p *CustomType) UnmarshalText(_ []byte) error {
	return errors.New("test err")
}

type A struct {
	B B `json:"b"`
}
type B struct {
	Cs []C `json:"cs"`
}
type C struct {
	D CustomType `json:"d"`
}

func main() {
	a := A{}
	err := json.Unmarshal([]byte(`{"b":{"cs":[{"d": "val"}]}}`), &a)
	fmt.Println(err)
	fmt.Printf("%#v\n", err)
}

output:

test err
&errors.errorString{s:"test err"}

This way, caller has no way to locate the error in input json when the input is a bit larger / contains multiple fields, some of them nested.

Compare to a similar error when a type in input json is not compatible with the struct field type:

https://go.dev/play/p/ZgexFrs-r_c

// same type definitions

func main() {
	a := A{}
	err := json.Unmarshal([]byte(`{"b":{"cs":[{"d": 0}]}}`), &a)
	fmt.Println(err)
	fmt.Printf("%#v\n", err)
}

output:

json: cannot unmarshal number into Go struct field C.b.cs.d of type main.CustomType
&json.UnmarshalTypeError{Value:"number", Type:(*reflect.rtype)(0x4b5460), Offset:19, Struct:"C", Field:"b.cs.d"}

In this case, the returned error contains a lot of useful information to locate the problem in intput, specifically a Field containing a path in json to the problematic value.

Proposal:

In a similar way to json.UnmarshalTypeError, when the json.Unmarshal gets an error from either .UnmarshalText() or .UnmarshalJSON(), the error would be wrapped into a similar structure, for example (currenlty not existing, proposed error) &json.UnmarshalerError{Type:(*reflect.rtype)(0x4b5460), Offset:19, Struct:"C", Field:"b.cs.d", Err: &errors.errorString{s:"test err"}}. Compared to json.UnmarshalTypeError, the Value is removed, as it might be not useful (and if needed can be added by the field type's unmarshal method), but Err with the original error is added, so that the original error is still Isable/Asable

(Also, the Field:"b.cs.d" could be Field:"b.cs[0].d" to better locate error in json lists, but to remain consistent with json.UnmarshalTypeError, it would be the same, that is Field:"b.cs.d")

Note

I see a very similar proposal was submitted very recently - returning a more friendly error, although in a different scenario (disallowed unknown field): #58649
So maybe the solution could be somewhat similar (for each case a special error type?) and could be possibly implemented together

[rittneje]

Unfortunately, this is a breaking change, since currently people could be doing an equality check (==) on the error that json.Unmarshal returns. (Yes they should be using errors.Is, but there is no guarantee they are, especially because their code could predate the existence of errors.Is.)

[earthboundkid]

See https://github.com/go-json-experiment/json in which some members of the Go team were working on an experiment to create a new version of encoding/json. ISTM as an outsider that an improvements to encoding/json will have to run through that package before getting into the standard library.

[Fazt01]

@carlmjohnson thanks for pointing me there. That is a large redesign, and I guess no one has any idea if/when that would be available in encoding/json(/v2).
It seems to plan for this particular issue, but is not yet implemented: same code but with json package replaced with "github.com/go-json-experiment/json" outputs:

json: cannot unmarshal JSON string into Go value of type main.CustomType: test err
&json.SemanticError{requireKeyedLiterals:json.requireKeyedLiterals{}, nonComparable:json.nonComparable{}, action:"unmarshal", ByteOffset:0, JSONPointer:"", JSONKind:0x22, GoType:(*reflect.rtype)(0x4ee120), Err:(*errors.errorString)(0x5d7210)}

The JSONPointer is currenlty empty, but from description is going to contain the path to problematic json field (including slice indexed)

@rittneje thanks for noting that, I didn't even know there were times without errors.Is 😄 . If it wasn't for that, I would personally even not consider it a breaking change (Is/As "should" be used).

With that, the only non-breaking change that comes to mind is adding a setting to json.Decoder similar to .DisallowUnknownFields() or UseNumber() (these are the only 2 settings) - something like .DetailedErrors() - the new behaviour of retunred errors would only happen with such setting. And also this new behaviour would not be available with json.Unmarshal

[dolmen]

This feature would be especially helpful for types implementing json.Unmarhaler and which use json.Unmarshal themself (recursively). It isn't currently possible to propagate and recontextualize the error location of an unmarshal error occuring in an inner json.Unmarshal.

The usefulness of my own package github.com/dolmen-go/jsonptrerror (which aims to enrich JSON unmarshal errors with a JSON Pointer) is quite limited because of this reason.