diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8a1927a39ca..d1899f3511f 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,3 +3,5 @@ ### SDK Enhancements ### SDK Bugs +* `restjson`: Correct failure to deserialize errors. + * Deserialize generic error information when no response body is present. diff --git a/private/protocol/restjson/unmarshal_error.go b/private/protocol/restjson/unmarshal_error.go index 4fffd0427ba..5366a646d9c 100644 --- a/private/protocol/restjson/unmarshal_error.go +++ b/private/protocol/restjson/unmarshal_error.go @@ -2,6 +2,7 @@ package restjson import ( "bytes" + "encoding/json" "io" "io/ioutil" "net/http" @@ -40,54 +41,30 @@ func (u *UnmarshalTypedError) UnmarshalError( resp *http.Response, respMeta protocol.ResponseMetadata, ) (error, error) { - - code := resp.Header.Get(errorTypeHeader) - msg := resp.Header.Get(errorMessageHeader) - - body := resp.Body - if len(code) == 0 || len(msg) == 0 { - // If unable to get code from HTTP headers have to parse JSON message - // to determine what kind of exception this will be. - var buf bytes.Buffer - var jsonErr jsonErrorResponse - teeReader := io.TeeReader(resp.Body, &buf) - err := jsonutil.UnmarshalJSONError(&jsonErr, teeReader) - if err != nil { - return nil, err - } - - body = ioutil.NopCloser(&buf) - if len(code) == 0 { - code = jsonErr.Code - } - msg = jsonErr.Message + code, msg, err := unmarshalErrorInfo(resp) + if err != nil { + return nil, err } - // If code has colon separators remove them so can compare against modeled - // exception names. - code = strings.SplitN(code, ":", 2)[0] - - if fn, ok := u.exceptions[code]; ok { - // 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.UnmarshalJSONCaseInsensitive(v, body); err != nil { - return nil, err - } + fn, ok := u.exceptions[code] + if !ok { + return awserr.NewRequestFailure( + awserr.New(code, msg, nil), + respMeta.StatusCode, + respMeta.RequestID, + ), nil + } - if err := rest.UnmarshalResponse(resp, v, true); err != nil { - return nil, err - } + v := fn(respMeta) + if err := jsonutil.UnmarshalJSONCaseInsensitive(v, resp.Body); err != nil { + return nil, err + } - return v, nil + if err := rest.UnmarshalResponse(resp, v, true); err != nil { + return nil, err } - // fallback to unmodeled generic exceptions - return awserr.NewRequestFailure( - awserr.New(code, msg, nil), - respMeta.StatusCode, - respMeta.RequestID, - ), nil + return v, nil } // UnmarshalErrorHandler is a named request handler for unmarshaling restjson @@ -101,36 +78,80 @@ var UnmarshalErrorHandler = request.NamedHandler{ func UnmarshalError(r *request.Request) { defer r.HTTPResponse.Body.Close() - var jsonErr jsonErrorResponse - err := jsonutil.UnmarshalJSONError(&jsonErr, r.HTTPResponse.Body) + code, msg, err := unmarshalErrorInfo(r.HTTPResponse) if err != nil { r.Error = awserr.NewRequestFailure( - awserr.New(request.ErrCodeSerialization, - "failed to unmarshal response error", err), + awserr.New(request.ErrCodeSerialization, "failed to unmarshal response error", err), r.HTTPResponse.StatusCode, r.RequestID, ) return } - code := r.HTTPResponse.Header.Get(errorTypeHeader) - if code == "" { - code = jsonErr.Code - } - msg := r.HTTPResponse.Header.Get(errorMessageHeader) - if msg == "" { - msg = jsonErr.Message - } - - code = strings.SplitN(code, ":", 2)[0] r.Error = awserr.NewRequestFailure( - awserr.New(code, jsonErr.Message, nil), + awserr.New(code, msg, nil), r.HTTPResponse.StatusCode, r.RequestID, ) } type jsonErrorResponse struct { + Type string `json:"__type"` Code string `json:"code"` Message string `json:"message"` } + +func (j *jsonErrorResponse) SanitizedCode() string { + code := j.Code + if len(j.Type) > 0 { + code = j.Type + } + return sanitizeCode(code) +} + +// Remove superfluous components from a restJson error code. +// - If a : character is present, then take only the contents before the +// first : character in the value. +// - If a # character is present, then take only the contents after the first +// # character in the value. +// +// All of the following error values resolve to FooError: +// - FooError +// - FooError:http://internal.amazon.com/coral/com.amazon.coral.validate/ +// - aws.protocoltests.restjson#FooError +// - aws.protocoltests.restjson#FooError:http://internal.amazon.com/coral/com.amazon.coral.validate/ +func sanitizeCode(code string) string { + noColon := strings.SplitN(code, ":", 2)[0] + hashSplit := strings.SplitN(noColon, "#", 2) + return hashSplit[len(hashSplit)-1] +} + +// attempt to garner error details from the response, preferring header values +// when present +func unmarshalErrorInfo(resp *http.Response) (code string, msg string, err error) { + code = sanitizeCode(resp.Header.Get(errorTypeHeader)) + msg = resp.Header.Get(errorMessageHeader) + if len(code) > 0 && len(msg) > 0 { + return + } + + // a modeled error will have to be re-deserialized later, so the body must + // be preserved + var buf bytes.Buffer + tee := io.TeeReader(resp.Body, &buf) + defer func() { resp.Body = ioutil.NopCloser(&buf) }() + + var jsonErr jsonErrorResponse + if decodeErr := json.NewDecoder(tee).Decode(&jsonErr); decodeErr != nil && decodeErr != io.EOF { + err = awserr.NewUnmarshalError(decodeErr, "failed to decode response body", buf.Bytes()) + return + } + + if len(code) == 0 { + code = jsonErr.SanitizedCode() + } + if len(msg) == 0 { + msg = jsonErr.Message + } + return +} diff --git a/private/protocol/restjson/unmarshal_error_test.go b/private/protocol/restjson/unmarshal_error_test.go index 9f1fbdc8895..7fd6dfae142 100644 --- a/private/protocol/restjson/unmarshal_error_test.go +++ b/private/protocol/restjson/unmarshal_error_test.go @@ -5,7 +5,6 @@ package restjson import ( "bytes" - "encoding/hex" "io/ioutil" "net/http" "reflect" @@ -18,11 +17,7 @@ import ( "github.com/aws/aws-sdk-go/private/protocol" ) -const unknownErrJSON = `{"code":"UnknownError", "message":"error message", "something":123}` -const simpleErrJSON = `{"code":"SimpleError", "message":"some message", "foo":123}` -const simpleNoCodeJSON = `{"message":"some message", "foo":123}` - -type SimpleError struct { +type SimpleModeledError struct { _ struct{} `type:"structure"` error @@ -30,19 +25,7 @@ type SimpleError struct { Foo *int64 `type:"integer" locationName:"foo"` } -const otherErrJSON = `{"code":"OtherError", "message":"some message"}` -const complexCodeErrJSON = `{"code":"OtherError:foo:bar", "message":"some message"}` - -type OtherError struct { - _ struct{} `type:"structure"` - error - - Message2 *string `type:"string" locationName:"message"` -} - -const complexErrJSON = `{"code":"ComplexError", "message":"some message", "foo": {"bar":"abc123", "baz":123}}` - -type ComplexError struct { +type ComplexModeledError struct { _ struct{} `type:"structure"` error @@ -51,6 +34,7 @@ type ComplexError struct { HeaderVal *string `type:"string" location:"header" locationName:"some-header"` Status *int64 `type:"integer" location:"statusCode"` } + type ErrorNested struct { _ struct{} `type:"structure"` @@ -59,7 +43,6 @@ type ErrorNested struct { } func TestUnmarshalTypedError(t *testing.T) { - respMeta := protocol.ResponseMetadata{ StatusCode: 400, RequestID: "abc123", @@ -67,13 +50,10 @@ func TestUnmarshalTypedError(t *testing.T) { exceptions := map[string]func(protocol.ResponseMetadata) error{ "SimpleError": func(meta protocol.ResponseMetadata) error { - return &SimpleError{} - }, - "OtherError": func(meta protocol.ResponseMetadata) error { - return &OtherError{} + return &SimpleModeledError{} }, "ComplexError": func(meta protocol.ResponseMetadata) error { - return &ComplexError{} + return &ComplexModeledError{} }, } @@ -82,44 +62,155 @@ func TestUnmarshalTypedError(t *testing.T) { Expect error Err string }{ - "simple error": { + "SimpleError": { + Response: &http.Response{ + Header: http.Header{ + errorTypeHeader: []string{"SimpleError"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, w/namespace": { + Response: &http.Response{ + Header: http.Header{ + errorTypeHeader: []string{"namespace#SimpleError"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, w/details": { + Response: &http.Response{ + Header: http.Header{ + errorTypeHeader: []string{"SimpleError:details:1"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, w/namespace+details": { + Response: &http.Response{ + Header: http.Header{ + errorTypeHeader: []string{"namespace#SimpleError:details:1"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, no header": { + Response: &http.Response{ + Header: http.Header{}, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, no header, body __type": { + Response: &http.Response{ + Header: http.Header{}, + Body: ioutil.NopCloser(strings.NewReader(`{"__type":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, no header, w/namespace": { Response: &http.Response{ Header: http.Header{}, - Body: ioutil.NopCloser(strings.NewReader(simpleErrJSON)), + Body: ioutil.NopCloser(strings.NewReader(`{"code":"namespace#SimpleError","message":"simple error message","foo":1}`)), }, - Expect: &SimpleError{ - Message2: aws.String("some message"), - Foo: aws.Int64(123), + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), }, }, - "other error": { + "SimpleError, no header, w/details": { Response: &http.Response{ Header: http.Header{}, - Body: ioutil.NopCloser(strings.NewReader(otherErrJSON)), + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError:details:1","message":"simple error message","foo":1}`)), }, - Expect: &OtherError{ - Message2: aws.String("some message"), + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), }, }, - "other complex Code error": { + "SimpleError, no header, w/namespace+details": { Response: &http.Response{ Header: http.Header{}, - Body: ioutil.NopCloser(strings.NewReader(complexCodeErrJSON)), + Body: ioutil.NopCloser(strings.NewReader(`{"code":"namespace#SimpleError:details:1","message":"simple error message","foo":1}`)), }, - Expect: &OtherError{ - Message2: aws.String("some message"), + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), }, }, - "complex error": { + "SimpleError, override message header": { + Response: &http.Response{ + Header: http.Header{ + errorMessageHeader: []string{"overriden error message"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"SimpleError","message":"simple error message","foo":1}`)), + }, + Expect: &SimpleModeledError{ + Message2: aws.String("simple error message"), + Foo: aws.Int64(1), + }, + }, + "SimpleError, no body": { + Response: &http.Response{ + Header: http.Header{ + errorTypeHeader: []string{"SimpleError"}, + errorMessageHeader: []string{"simple error message"}, + }, + Body: http.NoBody, + }, + Expect: &SimpleModeledError{}, + }, + "ComplexError": { + Response: &http.Response{ + StatusCode: 400, + Header: http.Header{ + errorTypeHeader: []string{"ComplexError"}, + "Some-Header": []string{"headval"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"ComplexError","message":"complex error message","foo":{"bar":"abc123","baz":123}}`)), + }, + Expect: &ComplexModeledError{ + Message2: aws.String("complex error message"), + HeaderVal: aws.String("headval"), + Status: aws.Int64(400), + Foo: &ErrorNested{ + Bar: aws.String("abc123"), + Baz: aws.Int64(123), + }, + }, + }, + "ComplexError, no type header": { Response: &http.Response{ StatusCode: 400, Header: http.Header{ "Some-Header": []string{"headval"}, }, - Body: ioutil.NopCloser(strings.NewReader(complexErrJSON)), + Body: ioutil.NopCloser(strings.NewReader(`{"code":"ComplexError","message":"complex error message","foo":{"bar":"abc123","baz":123}}`)), }, - Expect: &ComplexError{ - Message2: aws.String("some message"), + Expect: &ComplexModeledError{ + Message2: aws.String("complex error message"), HeaderVal: aws.String("headval"), Status: aws.Int64(400), Foo: &ErrorNested{ @@ -128,32 +219,46 @@ func TestUnmarshalTypedError(t *testing.T) { }, }, }, - "unknown error": { + "ComplexError, override message header": { Response: &http.Response{ - Header: http.Header{}, - Body: ioutil.NopCloser(strings.NewReader(unknownErrJSON)), + StatusCode: 400, + Header: http.Header{ + errorMessageHeader: []string{"overriden error message"}, + "Some-Header": []string{"headval"}, + }, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"ComplexError","message":"complex error message","foo":{"bar":"abc123","baz":123}}`)), + }, + Expect: &ComplexModeledError{ + Message2: aws.String("complex error message"), + HeaderVal: aws.String("headval"), + Status: aws.Int64(400), + Foo: &ErrorNested{ + Bar: aws.String("abc123"), + Baz: aws.Int64(123), + }, }, - Expect: awserr.NewRequestFailure( - awserr.New("UnknownError", "error message", nil), - respMeta.StatusCode, - respMeta.RequestID, - ), }, - "invalid error": { + "ComplexError, no body": { Response: &http.Response{ StatusCode: 400, - Header: http.Header{}, - Body: ioutil.NopCloser(strings.NewReader(`{`)), + Header: http.Header{ + errorTypeHeader: []string{"ComplexError"}, + "Some-Header": []string{"headval"}, + }, + Body: http.NoBody, + }, + Expect: &ComplexModeledError{ + Status: aws.Int64(400), + HeaderVal: aws.String("headval"), }, - Err: "failed decoding", }, - "unknown from header": { + "UnknownError": { Response: &http.Response{ Header: http.Header{ errorTypeHeader: []string{"UnknownError"}, errorMessageHeader: []string{"error message"}, }, - Body: ioutil.NopCloser(nil), + Body: http.NoBody, }, Expect: awserr.NewRequestFailure( awserr.New("UnknownError", "error message", nil), @@ -161,56 +266,76 @@ func TestUnmarshalTypedError(t *testing.T) { respMeta.RequestID, ), }, - "unknown code header only": { + "UnknownError, no message": { Response: &http.Response{ Header: http.Header{ errorTypeHeader: []string{"UnknownError"}, }, - Body: ioutil.NopCloser(strings.NewReader(unknownErrJSON)), + Body: http.NoBody, }, Expect: awserr.NewRequestFailure( - awserr.New("UnknownError", "error message", nil), + awserr.New("UnknownError", "", nil), respMeta.StatusCode, respMeta.RequestID, ), }, - "unknown message header only": { + "UnknownError, no header type, body __type": { Response: &http.Response{ - Header: http.Header{ - errorMessageHeader: []string{"overwritten error message"}, - }, - Body: ioutil.NopCloser(strings.NewReader(unknownErrJSON)), + Header: http.Header{}, + Body: ioutil.NopCloser(strings.NewReader(`{"__type":"UnknownError"}`)), }, Expect: awserr.NewRequestFailure( - awserr.New("UnknownError", "error message", nil), + awserr.New("UnknownError", "", nil), respMeta.StatusCode, respMeta.RequestID, ), }, - "code from header": { + "UnknownError, no header type, body code": { Response: &http.Response{ - Header: http.Header{ - errorTypeHeader: []string{"SimpleError"}, - }, - Body: ioutil.NopCloser(strings.NewReader(simpleNoCodeJSON)), + Header: http.Header{}, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"UnknownError"}`)), }, - Expect: &SimpleError{ - Message2: aws.String("some message"), - Foo: aws.Int64(123), + Expect: awserr.NewRequestFailure( + awserr.New("UnknownError", "", nil), + respMeta.StatusCode, + respMeta.RequestID, + ), + }, + "UnknownError, body only": { + Response: &http.Response{ + Header: http.Header{}, + Body: ioutil.NopCloser(strings.NewReader(`{"code":"UnknownError","message":"unknown error message"}`)), }, + Expect: awserr.NewRequestFailure( + awserr.New("UnknownError", "unknown error message", nil), + respMeta.StatusCode, + respMeta.RequestID, + ), }, - "ignore message header": { + "no code": { Response: &http.Response{ Header: http.Header{ - errorTypeHeader: []string{"SimpleError"}, - errorMessageHeader: []string{"error message"}, + errorMessageHeader: []string{"no code message"}, }, - Body: ioutil.NopCloser(strings.NewReader(simpleNoCodeJSON)), + Body: http.NoBody, }, - Expect: &SimpleError{ - Message2: aws.String("some message"), - Foo: aws.Int64(123), + Expect: awserr.NewRequestFailure( + awserr.New("", "no code message", nil), + respMeta.StatusCode, + respMeta.RequestID, + ), + }, + "no information": { + Response: &http.Response{ + StatusCode: 400, + Header: http.Header{}, + Body: http.NoBody, }, + Expect: awserr.NewRequestFailure( + awserr.New("", "", nil), + respMeta.StatusCode, + respMeta.RequestID, + ), }, } @@ -243,21 +368,6 @@ func TestUnmarshalError_SerializationError(t *testing.T) { ExpectMsg string ExpectBytes []byte }{ - "empty body": { - Request: &request.Request{ - Data: &struct{}{}, - HTTPResponse: &http.Response{ - StatusCode: 400, - Header: http.Header{ - "X-Amzn-Requestid": []string{"abc123"}, - }, - Body: ioutil.NopCloser( - bytes.NewReader([]byte{}), - ), - }, - }, - ExpectMsg: "error message missing", - }, "HTML body": { Request: &request.Request{ Data: &struct{}{}, @@ -272,7 +382,7 @@ func TestUnmarshalError_SerializationError(t *testing.T) { }, }, ExpectBytes: []byte(``), - ExpectMsg: "failed decoding", + ExpectMsg: "failed to decode response body", }, } @@ -294,9 +404,6 @@ func TestUnmarshalError_SerializationError(t *testing.T) { if e, a := c.ExpectMsg, uerr.Message(); !strings.Contains(a, e) { t.Errorf("Expect %q, in %q", e, a) } - if e, a := c.ExpectBytes, uerr.Bytes(); !bytes.Equal(e, a) { - t.Errorf("expect:\n%v\nactual:\n%v", hex.Dump(e), hex.Dump(a)) - } }) } }