Skip to content

Commit

Permalink
GODRIVER-1646: Unmarshal errors should return multiple errors when po…
Browse files Browse the repository at this point in the history 
…ssible

Return the decode errors to the library user rather than printing it to the console

Remove IgnoreDecodingError switch on the registry builder

Fix lint issues

Fix most tests, still issues to investigate with vr.skip()/dr.ReadElement() not implemented in bsonrwtest ...

Signed-off-by: Julien Spadoni <julien@beezeelinx.com>
  • Loading branch information
@Julien-Beezeelinx
Julien-Beezeelinx committed Feb 21, 2024
1 parent d0842bc commit a344922
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 60 deletions.
21 changes: 16 additions & 5 deletions bson/bsoncodec/bsoncodec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (tde TypeDecoderError) Error() string {
return fmt.Sprintf("cannot decode %s into a %s", tde.Received.String(), strings.Join(typeKinds, ", "))
}

// MultiDecodeError represents mutliple wrapped DecodeError that occurs when unmarshalling BSON bytes into a native Go type.
// MultiDecodeError represents multiple wrapped DecodeError that occurs when unmarshalling BSON bytes into a native Go type.
type MultiDecodeError struct {
wrapped []error
}
Expand Down Expand Up @@ -178,9 +178,20 @@ func (de *MultiDecodeError) setWrappedDecodeErrorKey(errKey string) {
}
}

// Create a Wrapped decoder error, setting the errKey on all child errors
// IsDecodeError returns the wrapped DecodeError and true if the passed error is a DecodeError
func IsDecodeError(err error) (error, bool) {
if _, ok := err.(*DecodeError); ok {
return err, true
} else if _, ok := err.(*MultiDecodeError); ok {
return err, true
} else {
return nil, false
}
}

// NewMultiDecodeError Create a Wrapped decoder error, setting the errKey on all child errors
// If there is only one child errors, return this one
func newMultiDecodeError(errKey string, childErrors ...error) error {
func NewMultiDecodeError(errKey string, childErrors ...error) error {
if len(childErrors) == 0 {
return nil
} else if len(childErrors) == 1 {
Expand Down Expand Up @@ -271,9 +282,9 @@ func (de *DecodeError) Error() string {
if de.keys != nil && len(de.keys) > 0 {
keyPath := strings.Join(de.Keys(), ".")
return fmt.Sprintf("error decoding key \"%s\" : %s", keyPath, de.Unwrap())
} else {
return fmt.Sprintf("decoding error : %s", de.Unwrap())
}

return fmt.Sprintf("decoding error : %s", de.Unwrap())
}

// Append a bson key name on a decode error
Expand Down
16 changes: 8 additions & 8 deletions bson/bsoncodec/default_value_decoders.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,15 @@ func (dvd DefaultValueDecoders) DDecodeValue(dc DecodeContext, vr bsonrw.ValueRe
// Pass false for convert because we don't need to call reflect.Value.Convert for tEmpty.
elem, err := decodeTypeOrValueWithInfo(decoder, tEmptyTypeDecoder, dc, elemVr, tEmpty, false)
if err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(key, err))
decodeErrors = append(decodeErrors, NewMultiDecodeError(key, err))
}
if elem.IsValid() {
elems = append(elems, primitive.E{Key: key, Value: elem.Interface()})
}
}

val.Set(reflect.ValueOf(elems))
return newMultiDecodeError("", decodeErrors...)
return NewMultiDecodeError("", decodeErrors...)
}

func (dvd DefaultValueDecoders) booleanDecodeType(_ DecodeContext, vr bsonrw.ValueReader, t reflect.Type) (reflect.Value, error) {
Expand Down Expand Up @@ -1593,13 +1593,13 @@ func (dvd DefaultValueDecoders) MapDecodeValue(dc DecodeContext, vr bsonrw.Value

err = decoder.DecodeValue(dc, vr, elem)
if err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(key, err))
decodeErrors = append(decodeErrors, NewMultiDecodeError(key, err))
}
if elem.IsValid() {
val.SetMapIndex(reflect.ValueOf(key).Convert(keyType), elem)
}
}
return newMultiDecodeError("", decodeErrors...)
return NewMultiDecodeError("", decodeErrors...)
}

// ArrayDecodeValue is the ValueDecoderFunc for array types.
Expand Down Expand Up @@ -1902,7 +1902,7 @@ func (dvd DefaultValueDecoders) decodeDefault(dc DecodeContext, vr bsonrw.ValueR

elem, err := decodeTypeOrValueWithInfo(decoder, eTypeDecoder, dc, vr, eType, true)
if err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError("["+strconv.Itoa(idx)+"]", err))
decodeErrors = append(decodeErrors, NewMultiDecodeError("["+strconv.Itoa(idx)+"]", err))
}
if elem.IsValid() {
elems = append(elems, elem)
Expand All @@ -1911,7 +1911,7 @@ func (dvd DefaultValueDecoders) decodeDefault(dc DecodeContext, vr bsonrw.ValueR
idx++
}

return elems, newMultiDecodeError("", decodeErrors...)
return elems, NewMultiDecodeError("", decodeErrors...)
}

func (dvd DefaultValueDecoders) readCodeWithScope(dc DecodeContext, vr bsonrw.ValueReader) (primitive.CodeWithScope, error) {
Expand Down Expand Up @@ -2036,11 +2036,11 @@ func (DefaultValueDecoders) decodeElemsFromDocumentReader(dc DecodeContext, dr b

val := reflect.New(tEmpty).Elem()
if err := decoder.DecodeValue(dc, vr, val); err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(key, err))
decodeErrors = append(decodeErrors, NewMultiDecodeError(key, err))
}
if val.IsValid() {
elems = append(elems, reflect.ValueOf(primitive.E{Key: key, Value: val.Interface()}))
}
}
return elems, newMultiDecodeError("", decodeErrors...)
return elems, NewMultiDecodeError("", decodeErrors...)
}
36 changes: 26 additions & 10 deletions bson/bsoncodec/default_value_decoders_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,12 @@ func TestDefaultValueDecoders(t *testing.T) {
bool(false),
nil,
&bsonrwtest.ValueReaderWriter{BSONType: bsontype.String},
bsonrwtest.Nothing,
fmt.Errorf("cannot decode %v into a boolean", bsontype.String),
bsonrwtest.Skip,
&TypeDecoderError{
Name: "BooleanDecodeType",
Kinds: []reflect.Kind{reflect.Bool},
Received: bsontype.String,
},
},
{
"fast path",
Expand Down Expand Up @@ -164,8 +168,12 @@ func TestDefaultValueDecoders(t *testing.T) {
0,
nil,
&bsonrwtest.ValueReaderWriter{BSONType: bsontype.String},
bsonrwtest.Nothing,
fmt.Errorf("cannot decode %v into an integer type", bsontype.String),
bsonrwtest.Skip,
&TypeDecoderError{
Name: "IntDecodeType",
Kinds: []reflect.Kind{reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int},
Received: bsontype.String,
},
},
{
"ReadInt32 error",
Expand Down Expand Up @@ -396,8 +404,12 @@ func TestDefaultValueDecoders(t *testing.T) {
0,
nil,
&bsonrwtest.ValueReaderWriter{BSONType: bsontype.String},
bsonrwtest.Nothing,
fmt.Errorf("cannot decode %v into an integer type", bsontype.String),
bsonrwtest.Skip,
&TypeDecoderError{
Name: "UintDecodeType",
Kinds: []reflect.Kind{reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uint},
Received: bsontype.String,
},
},
{
"ReadInt32 error",
Expand Down Expand Up @@ -632,8 +644,12 @@ func TestDefaultValueDecoders(t *testing.T) {
0,
nil,
&bsonrwtest.ValueReaderWriter{BSONType: bsontype.String},
bsonrwtest.Nothing,
fmt.Errorf("cannot decode %v into a float32 or float64 type", bsontype.String),
bsonrwtest.Skip,
&TypeDecoderError{
Name: "FloatDecodeType",
Kinds: []reflect.Kind{reflect.Float32, reflect.Float64},
Received: bsontype.String,
},
},
{
"ReadDouble error",
Expand Down Expand Up @@ -811,8 +827,8 @@ func TestDefaultValueDecoders(t *testing.T) {
map[bool]interface{}{},
&DecodeContext{Registry: buildDefaultRegistry()},
&bsonrwtest.ValueReaderWriter{},
bsonrwtest.ReadElement,
fmt.Errorf("unsupported key type: %T", false),
bsonrwtest.Skip,
&decodeMapKeyError{KeyType: reflect.TypeOf(false)},
},
{
"ReadDocument Error",
Expand Down
10 changes: 5 additions & 5 deletions bson/bsoncodec/map_codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,22 +234,22 @@ func (mc *MapCodec) DecodeValue(dc DecodeContext, vr bsonrw.ValueReader, val ref

k, err := mc.decodeKey(key, keyType)
if err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(key, err))
decodeErrors = append(decodeErrors, NewMultiDecodeError(key, err))
} else {
elem, err := decodeTypeOrValueWithInfo(decoder, eTypeDecoder, dc, vr, eType, true)
if err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(key, err))
decodeErrors = append(decodeErrors, NewMultiDecodeError(key, err))
}
if elem.IsValid() {
val.SetMapIndex(k, elem)
}
}
}
if len(decodeErrors) > 0 {
return newMultiDecodeError("", decodeErrors...)
} else {
return nil
return NewMultiDecodeError("", decodeErrors...)
}

return nil
}

func clearMap(m reflect.Value) {
Expand Down
8 changes: 4 additions & 4 deletions bson/bsoncodec/struct_codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ func (sc *StructCodec) DecodeValue(dc DecodeContext, vr bsonrw.ValueReader, val

if !field.CanSet() { // Being settable is a super set of being addressable.
innerErr := fmt.Errorf("field %v is not settable", field)
decodeErrors = append(decodeErrors, newMultiDecodeError(fd.name, innerErr))
decodeErrors = append(decodeErrors, NewMultiDecodeError(fd.name, innerErr))
} else {
if field.Kind() == reflect.Ptr && field.IsNil() {
field.Set(reflect.New(field.Type().Elem()))
Expand All @@ -359,17 +359,17 @@ func (sc *StructCodec) DecodeValue(dc DecodeContext, vr bsonrw.ValueReader, val
}

if fd.decoder == nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(fd.name, ErrNoDecoder{Type: field.Elem().Type()}))
decodeErrors = append(decodeErrors, NewMultiDecodeError(fd.name, ErrNoDecoder{Type: field.Elem().Type()}))
} else {
if err := fd.decoder.DecodeValue(dctx, vr, field.Elem()); err != nil {
decodeErrors = append(decodeErrors, newMultiDecodeError(fd.name, err))
decodeErrors = append(decodeErrors, NewMultiDecodeError(fd.name, err))
}
}

}
}

return newMultiDecodeError("", decodeErrors...)
return NewMultiDecodeError("", decodeErrors...)
}

func isEmpty(v reflect.Value, omitZeroStruct bool) bool {
Expand Down
22 changes: 1 addition & 21 deletions bson/decoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,27 +132,7 @@ func (d *Decoder) Decode(val interface{}) error {
}

err = decoder.DecodeValue(d.dc, d.vr, rval)
if err != nil {
if e, ok := err.(*bsoncodec.DecodeError); ok {
if d.dc.Registry.IgnoreDecodingError() {
fmt.Println(e)
return nil
} else {
return e
}
} else if e, ok := err.(*bsoncodec.MultiDecodeError); ok {
if d.dc.Registry.IgnoreDecodingError() {
fmt.Println(e)
return nil
} else {
return e
}
} else {
return err
}
} else {
return err
}
return err
}

// Reset will reset the state of the decoder, using the same *DecodeContext used in
Expand Down
2 changes: 0 additions & 2 deletions bson/mgocompat/registry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ func NewRegistryBuilder() *bsoncodec.RegistryBuilder {
bsoncodec.DefaultValueDecoders{}.RegisterDefaultDecoders(rb)
bson.PrimitiveCodecs{}.RegisterPrimitiveCodecs(rb)

rb.SetIgnoreDecodingError(true)

structcodec, _ := bsoncodec.NewStructCodec(bsoncodec.DefaultStructTagParser,
bsonoptions.StructCodec().
SetDecodeZeroStruct(true).
Expand Down
17 changes: 12 additions & 5 deletions mongo/cursor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,16 @@ func (c *Cursor) All(ctx context.Context, results interface{}) error {
// completes even if the context passed to All has errored.
defer c.Close(context.Background())

decodeErrors := make([]error, 0)
batch := c.batch // exhaust the current batch before iterating the batch cursor
for {
sliceVal, index, err = c.addFromBatch(sliceVal, elementType, batch, index)
if err != nil {
return err
if e, ok := bsoncodec.IsDecodeError(err); ok {
decodeErrors = append(decodeErrors, bsoncodec.NewMultiDecodeError("", e))
} else {
return err
}
}

if !c.bc.Next(ctx) {
Expand All @@ -333,7 +338,7 @@ func (c *Cursor) All(ctx context.Context, results interface{}) error {
}

resultsVal.Elem().Set(sliceVal.Slice(0, index))
return nil
return bsoncodec.NewMultiDecodeError("", decodeErrors...)
}

// RemainingBatchLength returns the number of documents left in the current batch. If this returns zero, the subsequent
Expand All @@ -352,6 +357,7 @@ func (c *Cursor) addFromBatch(sliceVal reflect.Value, elemType reflect.Type, bat
return sliceVal, index, err
}

decodeErrors := make([]error, 0)
for _, doc := range docs {
if sliceVal.Len() == index {
// slice is full
Expand All @@ -366,14 +372,15 @@ func (c *Cursor) addFromBatch(sliceVal reflect.Value, elemType reflect.Type, bat
return sliceVal, index, fmt.Errorf("error configuring BSON decoder: %w", err)
}
err = dec.Decode(currElem)
if err != nil {
if e, ok := bsoncodec.IsDecodeError(err); ok {
decodeErrors = append(decodeErrors, bsoncodec.NewMultiDecodeError("", e))
} else {
return sliceVal, index, err
}

index++
}

return sliceVal, index, nil
return sliceVal, index, bsoncodec.NewMultiDecodeError("", decodeErrors...)
}

func (c *Cursor) closeImplicitSession() {
Expand Down

0 comments on commit a344922

Please sign in to comment.