Skip to content

Commit

Permalink
feat: Add more context to decoding errors
Browse files Browse the repository at this point in the history
This feature adds field, line and column information
to the decoding error if available

Closes #43
  • Loading branch information
jszwec committed Aug 30, 2021
1 parent 4f1aebe commit 24d20bc
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 22 deletions.
14 changes: 14 additions & 0 deletions csvutil_go110_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package csvutil
import (
"encoding/csv"
"reflect"
"testing"
)

var testUnmarshalInvalidFirstLineErr = &csv.ParseError{
Expand All @@ -23,3 +24,16 @@ var testUnmarshalInvalidSecondLineErr = &csv.ParseError{
}

var ptrUnexportedEmbeddedDecodeErr = errPtrUnexportedStruct(reflect.TypeOf(new(embedded)))

func TestUnmarshalGo110(t *testing.T) {
t.Run("unmarshal type error message", func(t *testing.T) {
expected := `csvutil: cannot unmarshal "field" into Go value of type int: field "X"`
err := Unmarshal([]byte("Y,X\n1,1\n2,field"), &[]A{})
if err == nil {
t.Fatal("want err not to be nil")
}
if err.Error() != expected {
t.Errorf("want=%s; got %s", expected, err.Error())
}
})
}
14 changes: 14 additions & 0 deletions csvutil_go117_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package csvutil
import (
"encoding/csv"
"reflect"
"testing"
)

// In Go1.17 csv.ParseError.Column became 1-indexed instead of 0-indexed.
Expand All @@ -26,3 +27,16 @@ var testUnmarshalInvalidSecondLineErr = &csv.ParseError{
}

var ptrUnexportedEmbeddedDecodeErr = errPtrUnexportedStruct(reflect.TypeOf(new(embedded)))

func TestUnmarshalGo117(t *testing.T) {
t.Run("unmarshal type error message", func(t *testing.T) {
expected := `csvutil: cannot unmarshal "field" into Go value of type int: field "X" line 3 column 3`
err := Unmarshal([]byte("Y,X\n1,1\n2,field"), &[]A{})
if err == nil {
t.Fatal("want err not to be nil")
}
if err.Error() != expected {
t.Errorf("want=%s; got %s", expected, err.Error())
}
})
}
19 changes: 18 additions & 1 deletion csvutil_go17_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
//go:build !go1.10
// +build !go1.10

package csvutil

import "encoding/csv"
import (
"encoding/csv"
"testing"
)

var testUnmarshalInvalidFirstLineErr = &csv.ParseError{
Line: 1,
Expand All @@ -17,3 +21,16 @@ var testUnmarshalInvalidSecondLineErr = &csv.ParseError{
}

var ptrUnexportedEmbeddedDecodeErr error

func TestUnmarshalGo17(t *testing.T) {
t.Run("unmarshal type error message", func(t *testing.T) {
expected := `csvutil: cannot unmarshal "field" into Go value of type int: field "X"`
err := Unmarshal([]byte("Y,X\n1,1\n2,field"), &[]A{})
if err == nil {
t.Fatal("want err not to be nil")
}
if err.Error() != expected {
t.Errorf("want=%s; got %s", expected, err.Error())
}
})
}
74 changes: 59 additions & 15 deletions csvutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestUnmarshal(t *testing.T) {
for _, f := range fixtures {
t.Run(f.desc, func(t *testing.T) {
var a []A
if err := Unmarshal(f.data, &a); !reflect.DeepEqual(err, f.err) {
if err := Unmarshal(f.data, &a); !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}
})
Expand Down Expand Up @@ -432,7 +432,7 @@ func TestMarshal(t *testing.T) {
t.Run(f.desc, func(t *testing.T) {
b, err := Marshal(f.v)
if f.err != nil {
if !reflect.DeepEqual(f.err, err) {
if !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}
return
Expand Down Expand Up @@ -536,18 +536,6 @@ func TestMarshal(t *testing.T) {
})
}
})

t.Run("unmarshal type error message", func(t *testing.T) {
expected := `csvutil: cannot unmarshal "field" into Go value of type int`
err := Unmarshal([]byte("X\nfield"), &[]A{})
if err == nil {
t.Fatal("want err not to be nil")
}
if err.Error() != expected {
t.Errorf("want=%s; got %s", expected, err.Error())
}
})

}

type TypeJ struct {
Expand Down Expand Up @@ -819,7 +807,7 @@ func TestHeader(t *testing.T) {
t.Run(f.desc, func(t *testing.T) {
h, err := Header(f.v, f.tag)

if !reflect.DeepEqual(err, f.err) {
if !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}

Expand Down Expand Up @@ -884,3 +872,59 @@ func TestParity(t *testing.T) {
t.Errorf("want out=%v; got %v", in, out)
}
}

func checkErr(expected, err error) bool {
if expected == err {
return true
}

eVal := reflect.New(reflect.TypeOf(expected))
if !asError(err, eVal.Interface()) {
return false
}
return reflect.DeepEqual(eVal.Elem().Interface(), expected)
}

// asError is a copy of errors.As to support older Go versions.
//
// This copy exists because we want to avoid dependencies like:
// "golang.org/x/xerrors"
func asError(err error, target interface{}) bool {
if target == nil {
panic("errors: target cannot be nil")
}
val := reflect.ValueOf(target)
typ := val.Type()
if typ.Kind() != reflect.Ptr || val.IsNil() {
panic("errors: target must be a non-nil pointer")
}
targetType := typ.Elem()
if targetType.Kind() != reflect.Interface && !targetType.Implements(errorType) {
panic("errors: *target must be interface or implement error")
}
for err != nil {
if reflect.TypeOf(err).AssignableTo(targetType) {
val.Elem().Set(reflect.ValueOf(err))
return true
}
if x, ok := err.(interface{ As(interface{}) bool }); ok && x.As(target) {
return true
}
err = unwrap(err)
}
return false
}

var errorType = reflect.TypeOf((*error)(nil)).Elem()

// unwrap is a copy of errors.Unwrap for older Go versions and to avoid
// dependencies.
func unwrap(err error) error {
u, ok := err.(interface {
Unwrap() error
})
if !ok {
return nil
}
return u.Unwrap()
}
32 changes: 31 additions & 1 deletion decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,42 @@ fieldLoop:
}

if err := f.decodeFunc(s, fv); err != nil {
return err
return wrapDecodeError(d.r, d.header[f.columnIndex], f.columnIndex, err)
}
}
return nil
}

// wrapDecodeError provides the given error with more context such as:
// - column name (field)
// - line number
// - column within record
//
// Line and Column info is available only if the used Reader supports 'FieldPos'
// that is available e.g. in csv.Reader (since Go1.17).
//
// The caller should use errors.As in order to fetch the original error.
func wrapDecodeError(r Reader, field string, fieldIndex int, err error) error {
fp, ok := r.(interface {
FieldPos(fieldIndex int) (line, column int)
})
if !ok {
return &decodeError{
Field: field,
Err: err,
}
}

l, c := fp.FieldPos(fieldIndex)

return &decodeError{
Field: field,
Line: l,
Column: c,
Err: err,
}
}

func (d *Decoder) fields(k typeKey) ([]decField, error) {
if k == d.typeKey {
return d.cache, nil
Expand Down
4 changes: 2 additions & 2 deletions decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ string,"{""key"":""value""}"
{
desc: "slice of structs - pre-allocated",
in: "String,int\nfirst,1\nsecond,2",
out: &[]TypeI{0: TypeI{Int: 200}, 1024: TypeI{Int: 100}},
out: &[]TypeI{0: {Int: 200}, 1024: {Int: 100}},
expected: &[]TypeI{
{String: "first", Int: 1},
{String: "second", Int: 2},
Expand Down Expand Up @@ -1577,7 +1577,7 @@ string,"{""key"":""value""}"

err = r.Decode(&f.out)
if f.err != nil {
if !reflect.DeepEqual(f.err, err) {
if !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}
return
Expand Down
6 changes: 3 additions & 3 deletions encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ func TestEncoder(t *testing.T) {
for _, v := range f.in {
err := enc.Encode(v)
if f.err != nil {
if !reflect.DeepEqual(f.err, err) {
if !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}
return
Expand Down Expand Up @@ -1787,7 +1787,7 @@ func TestEncoder(t *testing.T) {
err := enc.EncodeHeader(f.in)
w.Flush()

if !reflect.DeepEqual(err, f.err) {
if !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}

Expand Down Expand Up @@ -2014,7 +2014,7 @@ func TestEncoder(t *testing.T) {
err := NewEncoder(w).Encode(f.in)

if f.err != nil {
if !reflect.DeepEqual(f.err, err) {
if !checkErr(f.err, err) {
t.Errorf("want err=%v; got %v", f.err, err)
}
return
Expand Down
23 changes: 23 additions & 0 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,26 @@ func (e *MissingColumnsError) Error() string {
}
return b.String()
}

// decodeError provides context to decoding errors if available.
//
// The caller should use errors.As in order to fetch the underlying error if
// needed.
type decodeError struct {
Field string
Line int
Column int
Err error
}

func (e *decodeError) Error() string {
if e.Line > 0 && e.Column > 0 {
// Lines and Columns are 1-indexed so this check is fine.
return fmt.Sprintf("%s: field %q line %d column %d", e.Err, e.Field, e.Line, e.Column)
}
return fmt.Sprintf("%s: field %q", e.Err, e.Field)
}

func (e *decodeError) Unwrap() error {
return e.Err
}

0 comments on commit 24d20bc

Please sign in to comment.