jsonpb: improve error reporting

.gitignore

@@ -13,5 +13,8 @@ _obj
 _test
 _testmain.go
 
+# Ignore Gogland stuff
+.idea/
+
 # Conformance test output and transient files.
 conformance/failing_tests.txt

jsonpb/errors.go

@@ -0,0 +1,95 @@
+// Go support for Protocol Buffers - Google's data interchange format
+//
+// Copyright 2015 The Go Authors.  All rights reserved.
+// https://github.com/golang/protobuf
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+package jsonpb
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"strings"
+
+	"github.com/golang/protobuf/proto"
+)
+
+type fieldError struct {
+	fieldStack []string
+	nestedErr  error
+}
+
+func (f *fieldError) Error() string {
+	return "unparsable field " + strings.Join(f.fieldStack, ".") + ": " + f.nestedErr.Error()
+}
+
+func (f *fieldError) FieldStack() []string {
+	return f.fieldStack
+}
+
+func (f *fieldError) UnderlyingError() error {
+	return f.nestedErr
+}
+
+// newFieldError wraps a given error providing a message call stack.
+func newFieldError(fieldName string, err error) error {
+	if fErr, ok := err.(*fieldError); ok {
+		fErr.fieldStack = append([]string{fieldName}, fErr.fieldStack...)
+		return err
+	}
+	return &fieldError{
+		fieldStack: []string{fieldName},
+		nestedErr:  err,
+	}
+}
+
+// correctJsonType gets rid of the dredded json.RawMessage errors and casts them to the right type.
+func correctJsonType(err error, realType reflect.Type) error {
+	if uErr, ok := err.(*json.UnmarshalTypeError); ok {
+		uErr.Type = realType
+		return uErr
+	}
+	return err
+}
+
+func getFieldMismatchError(remainingFields map[string]json.RawMessage, structProps *proto.StructProperties) error {
+	remaining := []string{}
+	for k, _ := range remainingFields {
+		remaining = append(remaining, k)
+	}
+	known := []string{}
+	for _, prop := range structProps.Prop {
+		jsonNames := acceptedJSONFieldNames(prop)
+		if strings.HasPrefix(jsonNames.camel, "XXX_") {
+			continue
+		}
+		known = append(known, jsonNames.camel)
+	}
+	return fmt.Errorf("fields %v do not exist in set of known fields %v", remaining, known)
+}

jsonpb/jsonpb.go

@@ -886,7 +886,7 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 	if targetType.Kind() == reflect.Struct {
 		var jsonFields map[string]json.RawMessage
 		if err := json.Unmarshal(inputValue, &jsonFields); err != nil {
-			return err
+			return correctJsonType(err, targetType)
 		}
 
 		consumeField := func(prop *proto.Properties) (json.RawMessage, bool) {
@@ -924,7 +924,7 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 			}
 
 			if err := u.unmarshalValue(target.Field(i), valueForField, sprops.Prop[i]); err != nil {
-				return err
+				return newFieldError(sprops.Prop[i].Name, err)
 			}
 		}
 		// Check for any oneof fields.
@@ -937,7 +937,7 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 				nv := reflect.New(oop.Type.Elem())
 				target.Field(oop.Field).Set(nv)
 				if err := u.unmarshalValue(nv.Elem().Field(0), raw, oop.Prop); err != nil {
-					return err
+					return newFieldError(oop.Prop.Name, err)
 				}
 			}
 		}
@@ -953,22 +953,16 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 					delete(jsonFields, name)
 					nv := reflect.New(reflect.TypeOf(ext.ExtensionType).Elem())
 					if err := u.unmarshalValue(nv.Elem(), raw, nil); err != nil {
-						return err
+						return newFieldError(name, err)
 					}
 					if err := proto.SetExtension(ep, ext, nv.Interface()); err != nil {
-						return err
+						return newFieldError(name, err)
 					}
 				}
 			}
 		}
 		if !u.AllowUnknownFields && len(jsonFields) > 0 {
-			// Pick any field to be the scapegoat.
-			var f string
-			for fname := range jsonFields {
-				f = fname
-				break
-			}
-			return fmt.Errorf("unknown field %q in %v", f, targetType)
+			return getFieldMismatchError(jsonFields, sprops)
 		}
 		return nil
 	}
@@ -977,14 +971,14 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 	if targetType.Kind() == reflect.Slice && targetType.Elem().Kind() != reflect.Uint8 {
 		var slc []json.RawMessage
 		if err := json.Unmarshal(inputValue, &slc); err != nil {
-			return err
+			return correctJsonType(err, targetType)
 		}
 		if slc != nil {
 			l := len(slc)
 			target.Set(reflect.MakeSlice(targetType, l, l))
 			for i := 0; i < l; i++ {
 				if err := u.unmarshalValue(target.Index(i), slc[i], prop); err != nil {
-					return err
+					return newFieldError(fmt.Sprintf("[%d]", i), err)
 				}
 			}
 		}
@@ -999,6 +993,12 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 		}
 		if mp != nil {
 			target.Set(reflect.MakeMap(targetType))
+			if prop != nil {
+				// These could still be nil if the protobuf metadata is broken somehow.
+				// TODO: This won't work because the fields are unexported.
+				// We should probably just reparse them.
+				//keyprop, valprop = prop.mkeyprop, prop.mvalprop
+			}
 			for ks, raw := range mp {
 				// Unmarshal map key. The core json library already decoded the key into a
 				// string, so we handle that specially. Other types were quoted post-serialization.
@@ -1009,15 +1009,15 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
 					k = reflect.New(targetType.Key()).Elem()
 					// TODO: pass the correct Properties if needed.
 					if err := u.unmarshalValue(k, json.RawMessage(ks), nil); err != nil {
-						return err
+						return newFieldError(fmt.Sprintf("['%s']", ks), newFieldError("key", err))
 					}
 				}
 
 				// Unmarshal map value.
 				v := reflect.New(targetType.Elem()).Elem()
 				// TODO: pass the correct Properties if needed.
 				if err := u.unmarshalValue(v, raw, nil); err != nil {
-					return err
+					return newFieldError(fmt.Sprintf("['%s']", ks), newFieldError("value", err))
 				}
 				target.SetMapIndex(k, v)
 			}

jsonpb/jsonpb_test.go

@@ -1148,3 +1148,67 @@ func TestUnmarshalUnsetRequiredFields(t *testing.T) {
 		}
 	}
 }
+
+
+func TestUnmarshalReturnsUsefulErrors(t *testing.T) {
+	for _, entry := range []struct {
+		name          string
+		input         string
+		msg           proto.Message
+		expectedError string
+	}{
+		{
+			name:          "FindsErrorsInNested",
+			input:         `{"simple": {"oString": 3.1}}`,
+			msg:           &pb.Widget{},
+			expectedError: `unparsable field Simple.OString: json: cannot unmarshal number into Go value of type string`,
+		},
+		{
+			name:          "FindsErrorsInArrays",
+			input:         `{"rInt32": [2,5,1,"sdas",2.1]}`,
+			msg:           &pb.Repeats{},
+			expectedError: `unparsable field RInt32.[3]: json: cannot unmarshal string into Go value of type int32`,
+		},
+		{
+			name:          "FindErrorsInMapKeys",
+			input:         `{"mInt64Str": {"1": "something", "badKey": "foo"}}`,
+			msg:           &pb.Maps{},
+			expectedError: `unparsable field MInt64Str.['badKey'].key: invalid character 'b' looking for beginning of value`,
+		},
+		{
+			name:          "FindErrorsInMapValues",
+			input:         `{"mInt64Str": {"1": "something", "2": 4.2}}`,
+			msg:           &pb.Maps{},
+			expectedError: `unparsable field MInt64Str.['2'].value: json: cannot unmarshal number into Go value of type string`,
+		},
+		{
+			name:          "HandlesGoodFormattingOfInt64AsString",
+			input:         `{"oInt64": "should_be_int"}`,
+			msg:           &pb.Simple{},
+			expectedError: `unparsable field OInt64: invalid character 's' looking for beginning of value`,
+		},
+		{
+			name:          "RemapsRawMessageToRealArrayType",
+			input:         `{"rInt32": "not_an_array"}`,
+			msg:           &pb.Repeats{},
+			expectedError: `unparsable field RInt32: json: cannot unmarshal string into Go value of type []int32`,
+		},
+		{
+			name:          "UnknownFieldErrors",
+			input:         `{"simple": {"oString": "something", "unknownOne": 1, "unknownTwo": 21}}`,
+			msg:           &pb.Widget{},
+			expectedError: `unparsable field Simple: fields [unknownOne unknownTwo] do not exist in set of known fields [oBool oInt32 oInt64 oUint32 oUint64 oSint32 oSint64 oFloat oDouble oString oBytes]`,
+		},
+	} {
+		u := Unmarshaler{AllowUnknownFields: false}
+		err := u.Unmarshal(strings.NewReader(entry.input), entry.msg)
+		if err == nil {
+			t.Errorf("in %v there should have been an error", entry.name)
+			continue
+		}
+		if err.Error() != entry.expectedError {
+			t.Errorf("in %v expected error \n'%v'\n but got \n'%v'", entry.name, entry.expectedError, err)
+			continue
+		}
+	}
+}