Skip to content

Commit

Permalink
Implement legacy error handling (#75)
Browse files Browse the repository at this point in the history 
In order to teach v2 how to mostly reproduce v1 errors,
add internal.{TransformMarshalError,NewMarshalerError,TransformUnmarshalError}
that can be called by v2 to transform a v2 error into a v1 error.
These injected functions are only ever called if ReportLegacyErrorValues is set.

TransformMarshalError and TransformUnmarshalError are called
in the top-level Marshal or Unmarshal functions if an error occurs.
NewMarshalerError is called if a user-defined Marshal method fails.

A JSONValue field is added to v2 SemanticError hold the entire copy
of an invalid JSON value during unmarshal. This is primarily populated
when trying to coerce a JSON number or string into a numeric value.
This field is needed to faithfully reproduce a v1 UnmarshalTypeError,
which sometimes holds the JSON value in the Value field.

The error positioning stored in UnmarshalTypeError was fixed
such that it is derived from the JSON pointer.
However, instead of being '/'-delimited, it remains '.'-delimited
to be consistent with historical precedence.
Unlike before, the position now includes Go array, slice, and map indexes.
Note that the positioning information in UnmarshalTypeError
has always been inconsistent and has also been unstable,
so hopefully changing this is okay.
  • Loading branch information
@dsnet
dsnet authored Dec 19, 2024
1 parent cad2278 commit fd80825
Show file tree
Hide file tree
Showing 17 changed files with 428 additions and 304 deletions.
47 changes: 30 additions & 17 deletions arshal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package json

import (
"bytes"
"errors"
"io"
"reflect"
"slices"
Expand Down Expand Up @@ -166,6 +165,9 @@ func Marshal(in any, opts ...Options) (out []byte, err error) {
xe := export.Encoder(enc)
xe.Flags.Set(jsonflags.OmitTopLevelNewline | 1)
err = marshalEncode(enc, in, &xe.Struct)
if err != nil && xe.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return nil, internal.TransformMarshalError(in, err)
}
return bytes.Clone(xe.Buf), err
}

Expand All @@ -178,7 +180,11 @@ func MarshalWrite(out io.Writer, in any, opts ...Options) (err error) {
defer export.PutStreamingEncoder(enc)
xe := export.Encoder(enc)
xe.Flags.Set(jsonflags.OmitTopLevelNewline | 1)
return marshalEncode(enc, in, &xe.Struct)
err = marshalEncode(enc, in, &xe.Struct)
if err != nil && xe.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.TransformMarshalError(in, err)
}
return err
}

// MarshalEncode serializes a Go value into an [jsontext.Encoder] according to
Expand All @@ -192,7 +198,11 @@ func MarshalEncode(out *jsontext.Encoder, in any, opts ...Options) (err error) {
mo.Join(opts...)
xe := export.Encoder(out)
mo.CopyCoderOptions(&xe.Struct)
return marshalEncode(out, in, mo)
err = marshalEncode(out, in, mo)
if err != nil && xe.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.TransformMarshalError(in, err)
}
return err
}

func marshalEncode(out *jsontext.Encoder, in any, mo *jsonopts.Struct) (err error) {
Expand Down Expand Up @@ -384,7 +394,11 @@ func Unmarshal(in []byte, out any, opts ...Options) (err error) {
dec := export.GetBufferedDecoder(in, opts...)
defer export.PutBufferedDecoder(dec)
xd := export.Decoder(dec)
return unmarshalFull(dec, out, &xd.Struct)
err = unmarshalFull(dec, out, &xd.Struct)
if err != nil && xd.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.TransformUnmarshalError(out, err)
}
return err
}

// UnmarshalRead deserializes a Go value from an [io.Reader] according to the
Expand All @@ -397,7 +411,11 @@ func UnmarshalRead(in io.Reader, out any, opts ...Options) (err error) {
dec := export.GetStreamingDecoder(in, opts...)
defer export.PutStreamingDecoder(dec)
xd := export.Decoder(dec)
return unmarshalFull(dec, out, &xd.Struct)
err = unmarshalFull(dec, out, &xd.Struct)
if err != nil && xd.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.TransformUnmarshalError(out, err)
}
return err
}

func unmarshalFull(in *jsontext.Decoder, out any, uo *jsonopts.Struct) error {
Expand Down Expand Up @@ -425,22 +443,17 @@ func UnmarshalDecode(in *jsontext.Decoder, out any, opts ...Options) (err error)
uo.Join(opts...)
xd := export.Decoder(in)
uo.CopyCoderOptions(&xd.Struct)
return unmarshalDecode(in, out, uo)
err = unmarshalDecode(in, out, uo)
if err != nil && uo.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.TransformUnmarshalError(out, err)
}
return err
}

var errNonNilReference = errors.New("value must be passed as a non-nil pointer reference")

func unmarshalDecode(in *jsontext.Decoder, out any, uo *jsonopts.Struct) (err error) {
v := reflect.ValueOf(out)
if !v.IsValid() || v.Kind() != reflect.Pointer || v.IsNil() {
var t reflect.Type
if v.IsValid() {
t = v.Type()
if t.Kind() == reflect.Pointer {
t = t.Elem()
}
}
return &SemanticError{action: "unmarshal", GoType: t, Err: errNonNilReference}
if v.Kind() != reflect.Pointer || v.IsNil() {
return &SemanticError{action: "unmarshal", GoType: reflect.TypeOf(out), Err: internal.ErrNonNilReference}
}
va := addressableValue{v.Elem()} // dereferenced pointer is always addressable
t := va.Type()
Expand Down
2 changes: 1 addition & 1 deletion arshal_any.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func unmarshalValueAny(dec *jsontext.Decoder, uo *jsonopts.Struct) (any, error)
}
fv, ok := jsonwire.ParseFloat(val, 64)
if !ok && uo.Flags.Get(jsonflags.RejectFloatOverflow) {
return nil, newUnmarshalErrorAfter(dec, float64Type, strconv.ErrRange)
return nil, newUnmarshalErrorAfterWithValue(dec, float64Type, strconv.ErrRange)
}
return fv, nil
default:
Expand Down
32 changes: 13 additions & 19 deletions arshal_default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,12 @@ type typedPointer struct {
len int // remember slice length to avoid false positives
}

var errCycle = errors.New("encountered a cycle")

// visitPointer visits pointer p of type t, reporting an error if seen before.
// If successfully visited, then the caller must eventually call leave.
func visitPointer(m *seenPointers, v reflect.Value) error {
p := typedPointer{v.Type(), v.UnsafePointer(), sliceLen(v)}
if _, ok := (*m)[p]; ok {
return errCycle
return internal.ErrCycle
}
if *m == nil {
*m = make(seenPointers)
Expand Down Expand Up @@ -174,7 +172,7 @@ func makeBoolArshaler(t reflect.Type) *arshaler {
case "false":
va.SetBool(false)
default:
return newUnmarshalErrorAfter(dec, t, fmt.Errorf("cannot parse %q as bool", tok.String()))
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrSyntax)
}
return nil
}
Expand Down Expand Up @@ -370,7 +368,7 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
b := va.Bytes()
if va.Kind() == reflect.Array {
if n != len(b) {
err := fmt.Errorf("decoded base64 length of %d mismatches array length of %d", n, len(b))
err := fmt.Errorf("decoded length of %d mismatches array length of %d", n, len(b))
return newUnmarshalErrorAfter(dec, t, err)
}
} else {
Expand All @@ -386,7 +384,8 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
// specifies that non-alphabet characters must be rejected.
// Unfortunately, the "base32" and "base64" packages allow
// '\r' and '\n' characters by default.
err = errors.New("illegal data at input byte " + strconv.Itoa(bytes.IndexAny(val, "\r\n")))
i := bytes.IndexAny(val, "\r\n")
err = fmt.Errorf("illegal character %s at offset %d", jsonwire.QuoteRune(val[i:]), i)
}
if err != nil {
return newUnmarshalErrorAfter(dec, t, err)
Expand All @@ -396,7 +395,7 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
}
return nil
}
return newUnmarshalErrorAfter(dec, t, err)
return newUnmarshalErrorAfter(dec, t, nil)
}
return fncs
}
Expand Down Expand Up @@ -460,14 +459,12 @@ func makeIntArshaler(t reflect.Type) *arshaler {
overflow := (neg && n > maxInt) || (!neg && n > maxInt-1)
if !ok {
if n != math.MaxUint64 {
err := fmt.Errorf("cannot parse %q as signed integer: %w", val, strconv.ErrSyntax)
return newUnmarshalErrorAfter(dec, t, err)
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrSyntax)
}
overflow = true
}
if overflow {
err := fmt.Errorf("cannot parse %q as signed integer: %w", val, strconv.ErrRange)
return newUnmarshalErrorAfter(dec, t, err)
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrRange)
}
if neg {
va.SetInt(int64(-n))
Expand Down Expand Up @@ -535,14 +532,12 @@ func makeUintArshaler(t reflect.Type) *arshaler {
overflow := n > maxUint-1
if !ok {
if n != math.MaxUint64 {
err := fmt.Errorf("cannot parse %q as unsigned integer: %w", val, strconv.ErrSyntax)
return newUnmarshalErrorAfter(dec, t, err)
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrSyntax)
}
overflow = true
}
if overflow {
err := fmt.Errorf("cannot parse %q as unsigned integer: %w", val, strconv.ErrRange)
return newUnmarshalErrorAfter(dec, t, err)
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrRange)
}
va.SetUint(n)
return nil
Expand All @@ -569,7 +564,7 @@ func makeFloatArshaler(t reflect.Type) *arshaler {
fv := va.Float()
if math.IsNaN(fv) || math.IsInf(fv, 0) {
if !allowNonFinite {
err := fmt.Errorf("invalid value: %v", fv)
err := fmt.Errorf("unsupported value: %v", fv)
return newMarshalErrorBefore(enc, t, err)
}
return enc.WriteToken(jsontext.Float(fv))
Expand Down Expand Up @@ -629,8 +624,7 @@ func makeFloatArshaler(t reflect.Type) *arshaler {
break
}
if n, err := jsonwire.ConsumeNumber(val); n != len(val) || err != nil {
err := fmt.Errorf("cannot parse %q as JSON number: %w", val, strconv.ErrSyntax)
return newUnmarshalErrorAfter(dec, t, err)
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrSyntax)
}
fallthrough
case '0':
Expand All @@ -639,7 +633,7 @@ func makeFloatArshaler(t reflect.Type) *arshaler {
}
fv, ok := jsonwire.ParseFloat(val, bits)
if !ok && uo.Flags.Get(jsonflags.RejectFloatOverflow) {
return newUnmarshalErrorAfter(dec, t, strconv.ErrRange)
return newUnmarshalErrorAfterWithValue(dec, t, strconv.ErrRange)
}
va.SetFloat(fv)
return nil
Expand Down
13 changes: 13 additions & 0 deletions arshal_funcs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"reflect"
"sync"

"github.com/go-json-experiment/json/internal"
"github.com/go-json-experiment/json/internal/jsonflags"
"github.com/go-json-experiment/json/internal/jsonopts"
"github.com/go-json-experiment/json/jsontext"
Expand Down Expand Up @@ -177,6 +178,9 @@ func MarshalFuncV1[T any](fn func(T) ([]byte, error)) *Marshalers {
val, err := fn(va.castTo(t).Interface().(T))
if err != nil {
err = wrapSkipFunc(err, "marshal function of type func(T) ([]byte, error)")
if export.Encoder(enc).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.NewMarshalerError(va.Addr().Interface(), err, "MarshalFuncV1") // unlike unmarshal, always wrapped
}
err = newMarshalErrorBefore(enc, t, err)
return collapseSemanticErrors(err)
}
Expand Down Expand Up @@ -226,6 +230,9 @@ func MarshalFuncV2[T any](fn func(*jsontext.Encoder, T, Options) error) *Marshal
}
err = errSkipMutation
}
if xe.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.NewMarshalerError(va.Addr().Interface(), err, "MarshalFuncV2") // unlike unmarshal, always wrapped
}
if !export.IsIOError(err) {
err = newSemanticErrorWithPosition(enc, t, prevDepth, prevLength, err)
}
Expand Down Expand Up @@ -260,6 +267,9 @@ func UnmarshalFuncV1[T any](fn func([]byte, T) error) *Unmarshalers {
err = fn(val, va.castTo(t).Interface().(T))
if err != nil {
err = wrapSkipFunc(err, "unmarshal function of type func([]byte, T) error")
if export.Decoder(dec).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return err // unlike marshal, never wrapped
}
err = newUnmarshalErrorAfter(dec, t, err)
return collapseSemanticErrors(err)
}
Expand Down Expand Up @@ -302,6 +312,9 @@ func UnmarshalFuncV2[T any](fn func(*jsontext.Decoder, T, Options) error) *Unmar
}
err = errSkipMutation
}
if export.Decoder(dec).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return err // unlike marshal, never wrapped
}
if !isSyntacticError(err) && !export.IsIOError(err) {
err = newSemanticErrorWithPosition(dec, t, prevDepth, prevLength, err)
}
Expand Down
22 changes: 22 additions & 0 deletions arshal_methods.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"reflect"

"github.com/go-json-experiment/json/internal"
"github.com/go-json-experiment/json/internal/jsonflags"
"github.com/go-json-experiment/json/internal/jsonopts"
"github.com/go-json-experiment/json/internal/jsonwire"
Expand Down Expand Up @@ -124,6 +125,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
}
if err != nil {
err = wrapSkipFunc(err, "marshal method")
if xe.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.NewMarshalerError(va.Addr().Interface(), err, "MarshalJSONV2") // unlike unmarshal, always wrapped
}
if !export.IsIOError(err) {
err = newSemanticErrorWithPosition(enc, t, prevDepth, prevLength, err)
}
Expand All @@ -138,6 +142,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
val, err := marshaler.MarshalJSON()
if err != nil {
err = wrapSkipFunc(err, "marshal method")
if export.Encoder(enc).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.NewMarshalerError(va.Addr().Interface(), err, "MarshalJSON") // unlike unmarshal, always wrapped
}
err = newMarshalErrorBefore(enc, t, err)
return collapseSemanticErrors(err)
}
Expand All @@ -155,6 +162,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
appender := va.Addr().Interface().(encodingTextAppender)
if err := export.Encoder(enc).AppendRaw('"', false, appender.AppendText); err != nil {
err = wrapSkipFunc(err, "append method")
if export.Encoder(enc).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.NewMarshalerError(va.Addr().Interface(), err, "AppendText") // unlike unmarshal, always wrapped
}
if !isSemanticError(err) && !export.IsIOError(err) {
err = newMarshalErrorBefore(enc, t, err)
}
Expand All @@ -171,6 +181,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
return append(b, b2...), err
}); err != nil {
err = wrapSkipFunc(err, "marshal method")
if export.Encoder(enc).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return internal.NewMarshalerError(va.Addr().Interface(), err, "MarshalText") // unlike unmarshal, always wrapped
}
if !isSemanticError(err) && !export.IsIOError(err) {
err = newMarshalErrorBefore(enc, t, err)
}
Expand Down Expand Up @@ -211,6 +224,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
}
if err != nil {
err = wrapSkipFunc(err, "unmarshal method")
if xd.Flags.Get(jsonflags.ReportLegacyErrorValues) {
return err // unlike marshal, never wrapped
}
if !isSyntacticError(err) && !export.IsIOError(err) {
err = newSemanticErrorWithPosition(dec, t, prevDepth, prevLength, err)
}
Expand All @@ -228,6 +244,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
unmarshaler := va.Addr().Interface().(UnmarshalerV1)
if err := unmarshaler.UnmarshalJSON(val); err != nil {
err = wrapSkipFunc(err, "unmarshal method")
if export.Decoder(dec).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return err // unlike marshal, never wrapped
}
err = newUnmarshalErrorAfter(dec, t, err)
return collapseSemanticErrors(err)
}
Expand All @@ -249,6 +268,9 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
unmarshaler := va.Addr().Interface().(encoding.TextUnmarshaler)
if err := unmarshaler.UnmarshalText(s); err != nil {
err = wrapSkipFunc(err, "unmarshal method")
if export.Decoder(dec).Flags.Get(jsonflags.ReportLegacyErrorValues) {
return err // unlike marshal, never wrapped
}
if !isSemanticError(err) && !isSyntacticError(err) && !export.IsIOError(err) {
err = newUnmarshalErrorAfter(dec, t, err)
}
Expand Down
Loading

0 comments on commit fd80825

Please sign in to comment.