Fix generated errors for some JSON APIs not including message (#3089)

Fixes the SDK's generated errors to all include the `Message` member regardless if it was modeled on the error shape. This fixes the bug identified in #3088 where some JSON errors were not modeled with the Message member. This caused the errors message to be dropped, and not retrievable. This change also updates the SDK's generated smoke test to expect the Message member of an error unmarshaled not to be empty. Fixes #3088
aws · Jan 21, 2020 · 6ca8a54 · 6ca8a54
1 parent 4c5dc88
commit 6ca8a54
Showing 108 changed files with 3,490 additions and 16 deletions.
diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
@@ -3,3 +3,5 @@
 ### SDK Enhancements
 
 ### SDK Bugs
+* Fix generated errors for some JSON APIs not including a message ([#3089](https://github.com/aws/aws-sdk-go/issues/3089))
+  * Fixes the SDK's generated errors to all include the `Message` member regardless if it was modeled on the error shape. This fixes the bug identified in #3088 where some JSON errors were not modeled with the Message member.
diff --git a/private/model/api/codegentest/service/restjsonservice/api.go b/private/model/api/codegentest/service/restjsonservice/api.go
diff --git a/private/model/api/codegentest/service/restxmlservice/api.go b/private/model/api/codegentest/service/restxmlservice/api.go
diff --git a/private/model/api/codegentest/service/rpcservice/api.go b/private/model/api/codegentest/service/rpcservice/api.go
diff --git a/private/model/api/load.go b/private/model/api/load.go
@@ -203,6 +203,7 @@ func (a *API) Setup() error {
 	a.setMetadataEndpointsKey()
 	a.writeShapeNames()
 	a.resolveReferences()
+	a.backfillErrorMembers()
 
 	if !a.NoRemoveUnusedShapes {
 		a.removeUnusedShapes()

diff --git a/private/model/api/passes.go b/private/model/api/passes.go
@@ -89,6 +89,47 @@ func (a *API) resolveReferences() {
 	}
 }
 
+func (a *API) backfillErrorMembers() {
+	stubShape := &Shape{
+		ShapeName: "string",
+		Type:      "string",
+	}
+	var locName string
+	switch a.Metadata.Protocol {
+	case "ec2", "query", "rest-xml":
+		locName = "Message"
+	case "json", "rest-json":
+		locName = "message"
+	}
+
+	for _, s := range a.Shapes {
+		if !s.Exception {
+			continue
+		}
+
+		var haveMessage bool
+		for name := range s.MemberRefs {
+			if strings.EqualFold(name, "Message") {
+				haveMessage = true
+				break
+			}
+		}
+		if !haveMessage {
+			ref := &ShapeRef{
+				ShapeName:    stubShape.ShapeName,
+				Shape:        stubShape,
+				LocationName: locName,
+			}
+			s.MemberRefs["Message"] = ref
+			stubShape.refs = append(stubShape.refs, ref)
+		}
+	}
+
+	if len(stubShape.refs) != 0 {
+		a.Shapes["SDKStubErrorMessageShape"] = stubShape
+	}
+}
+
 // A referenceResolver provides a way to resolve shape references to
 // shape definitions.
 type referenceResolver struct {

diff --git a/private/model/api/smoke.go b/private/model/api/smoke.go
@@ -113,6 +113,9 @@ var smokeTestTmpl = template.Must(template.New(`smokeTestTmpl`).Parse(`
 			if len(aerr.Code()) == 0 {
 				t.Errorf("expect non-empty error code")
 			}
+			if len(aerr.Message()) == 0 {
+				t.Errorf("expect non-empty error message")
+			}
 			if v := aerr.Code(); v == request.ErrCodeSerialization {
 				t.Errorf("expect API error code got serialization failure")
 			}

diff --git a/private/protocol/json/jsonutil/unmarshal.go b/private/protocol/json/jsonutil/unmarshal.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"reflect"
+	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -45,10 +46,31 @@ func UnmarshalJSON(v interface{}, stream io.Reader) error {
 		return err
 	}
 
-	return unmarshalAny(reflect.ValueOf(v), out, "")
+	return unmarshaler{}.unmarshalAny(reflect.ValueOf(v), out, "")
 }
 
-func unmarshalAny(value reflect.Value, data interface{}, tag reflect.StructTag) error {
+// UnmarshalJSONCaseInsensitive reads a stream and unmarshals the result into the
+// object v. Ignores casing for structure members.
+func UnmarshalJSONCaseInsensitive(v interface{}, stream io.Reader) error {
+	var out interface{}
+
+	err := json.NewDecoder(stream).Decode(&out)
+	if err == io.EOF {
+		return nil
+	} else if err != nil {
+		return err
+	}
+
+	return unmarshaler{
+		caseInsensitive: true,
+	}.unmarshalAny(reflect.ValueOf(v), out, "")
+}
+
+type unmarshaler struct {
+	caseInsensitive bool
+}
+
+func (u unmarshaler) unmarshalAny(value reflect.Value, data interface{}, tag reflect.StructTag) error {
 	vtype := value.Type()
 	if vtype.Kind() == reflect.Ptr {
 		vtype = vtype.Elem() // check kind of actual element type
@@ -80,17 +102,17 @@ func unmarshalAny(value reflect.Value, data interface{}, tag reflect.StructTag)
 		if field, ok := vtype.FieldByName("_"); ok {
 			tag = field.Tag
 		}
-		return unmarshalStruct(value, data, tag)
+		return u.unmarshalStruct(value, data, tag)
 	case "list":
-		return unmarshalList(value, data, tag)
+		return u.unmarshalList(value, data, tag)
 	case "map":
-		return unmarshalMap(value, data, tag)
+		return u.unmarshalMap(value, data, tag)
 	default:
-		return unmarshalScalar(value, data, tag)
+		return u.unmarshalScalar(value, data, tag)
 	}
 }
 
-func unmarshalStruct(value reflect.Value, data interface{}, tag reflect.StructTag) error {
+func (u unmarshaler) unmarshalStruct(value reflect.Value, data interface{}, tag reflect.StructTag) error {
 	if data == nil {
 		return nil
 	}
@@ -114,7 +136,7 @@ func unmarshalStruct(value reflect.Value, data interface{}, tag reflect.StructTa
 	// unwrap any payloads
 	if payload := tag.Get("payload"); payload != "" {
 		field, _ := t.FieldByName(payload)
-		return unmarshalAny(value.FieldByName(payload), data, field.Tag)
+		return u.unmarshalAny(value.FieldByName(payload), data, field.Tag)
 	}
 
 	for i := 0; i < t.NumField(); i++ {
@@ -128,17 +150,27 @@ func unmarshalStruct(value reflect.Value, data interface{}, tag reflect.StructTa
 		if locName := field.Tag.Get("locationName"); locName != "" {
 			name = locName
 		}
+		if u.caseInsensitive {
+			if _, ok := mapData[name]; !ok {
+				// Fallback to uncased name search if the exact name didn't match.
+				for kn, v := range mapData {
+					if strings.EqualFold(kn, name) {
+						mapData[name] = v
+					}
+				}
+			}
+		}
 
 		member := value.FieldByIndex(field.Index)
-		err := unmarshalAny(member, mapData[name], field.Tag)
+		err := u.unmarshalAny(member, mapData[name], field.Tag)
 		if err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func unmarshalList(value reflect.Value, data interface{}, tag reflect.StructTag) error {
+func (u unmarshaler) unmarshalList(value reflect.Value, data interface{}, tag reflect.StructTag) error {
 	if data == nil {
 		return nil
 	}
@@ -153,7 +185,7 @@ func unmarshalList(value reflect.Value, data interface{}, tag reflect.StructTag)
 	}
 
 	for i, c := range listData {
-		err := unmarshalAny(value.Index(i), c, "")
+		err := u.unmarshalAny(value.Index(i), c, "")
 		if err != nil {
 			return err
 		}
@@ -162,7 +194,7 @@ func unmarshalList(value reflect.Value, data interface{}, tag reflect.StructTag)
 	return nil
 }
 
-func unmarshalMap(value reflect.Value, data interface{}, tag reflect.StructTag) error {
+func (u unmarshaler) unmarshalMap(value reflect.Value, data interface{}, tag reflect.StructTag) error {
 	if data == nil {
 		return nil
 	}
@@ -179,14 +211,14 @@ func unmarshalMap(value reflect.Value, data interface{}, tag reflect.StructTag)
 		kvalue := reflect.ValueOf(k)
 		vvalue := reflect.New(value.Type().Elem()).Elem()
 
-		unmarshalAny(vvalue, v, "")
+		u.unmarshalAny(vvalue, v, "")
 		value.SetMapIndex(kvalue, vvalue)
 	}
 
 	return nil
 }
 
-func unmarshalScalar(value reflect.Value, data interface{}, tag reflect.StructTag) error {
+func (u unmarshaler) unmarshalScalar(value reflect.Value, data interface{}, tag reflect.StructTag) error {
 
 	switch d := data.(type) {
 	case nil:

diff --git a/private/protocol/jsonrpc/unmarshal_error.go b/private/protocol/jsonrpc/unmarshal_error.go
@@ -54,7 +54,7 @@ func (u *UnmarshalTypedError) UnmarshalError(
 		// If exception code is know, use associated constructor to get a value
 		// for the exception that the JSON body can be unmarshaled into.
 		v := fn(respMeta)
-		err := jsonutil.UnmarshalJSON(v, body)
+		err := jsonutil.UnmarshalJSONCaseInsensitive(v, body)
 		if err != nil {
 			return nil, err
 		}

diff --git a/private/protocol/jsonrpc/unmarshal_error_test.go b/private/protocol/jsonrpc/unmarshal_error_test.go
@@ -19,6 +19,7 @@ import (
 
 const unknownErrJSON = `{"__type":"UnknownError", "message":"error message", "something":123}`
 const simpleErrJSON = `{"__type":"SimpleError", "message":"some message", "foo":123}`
+const simpleCasedErrJSON = `{"__type":"SimpleError", "Message":"some message", "foo":123}`
 
 type SimpleError struct {
 	_ struct{} `type:"structure"`
@@ -138,6 +139,16 @@ func TestUnmarshalTypedError(t *testing.T) {
 			},
 			Err: "failed decoding",
 		},
+		"mixed case fields": {
+			Response: &http.Response{
+				Header: http.Header{},
+				Body:   ioutil.NopCloser(strings.NewReader(simpleCasedErrJSON)),
+			},
+			Expect: &SimpleError{
+				Message2: aws.String("some message"),
+				Foo:      aws.Int64(123),
+			},
+		},
 	}
 
 	for name, c := range cases {

diff --git a/private/protocol/restjson/unmarshal_error.go b/private/protocol/restjson/unmarshal_error.go
@@ -69,7 +69,7 @@ func (u *UnmarshalTypedError) UnmarshalError(
 		// If exception code is know, use associated constructor to get a value
 		// for the exception that the JSON body can be unmarshaled into.
 		v := fn(respMeta)
-		if err := jsonutil.UnmarshalJSON(v, body); err != nil {
+		if err := jsonutil.UnmarshalJSONCaseInsensitive(v, body); err != nil {
 			return nil, err
 		}
 

diff --git a/service/acm/integ_test.go b/service/acm/integ_test.go
diff --git a/service/apigateway/integ_test.go b/service/apigateway/integ_test.go