service/route53: Add batched error support for ChangeResourceRecordSets

Adds support for batched errors that ChangeResourceRecordSets can
optionally return.

Fix aws#438

Unmarshaling and tests provided by @abustany

service/route53/service.go

@@ -62,7 +62,9 @@ func newClient(cfg aws.Config, handlers request.Handlers, endpoint, signingRegio
 	svc.Handlers.Build.PushBackNamed(restxml.BuildHandler)
 	svc.Handlers.Unmarshal.PushBackNamed(restxml.UnmarshalHandler)
 	svc.Handlers.UnmarshalMeta.PushBackNamed(restxml.UnmarshalMetaHandler)
-	svc.Handlers.UnmarshalError.PushBackNamed(restxml.UnmarshalErrorHandler)
+
+	// Route53 uses a custom error parser
+	svc.Handlers.UnmarshalError.PushBack(unmarshalError)
 
 	// Run custom client initialization if present
 	if initClient != nil {

service/route53/unmarshal_error.go

@@ -0,0 +1,86 @@
+package route53
+
+import (
+	"bytes"
+	"encoding/xml"
+	"io/ioutil"
+
+	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/aws/aws-sdk-go/aws/request"
+	"github.com/aws/aws-sdk-go/private/protocol/restxml"
+)
+
+type baseXMLErrorResponse struct {
+	XMLName xml.Name
+}
+
+type standardXMLErrorResponse struct {
+	XMLName   xml.Name `xml:"ErrorResponse"`
+	Code      string   `xml:"Error>Code"`
+	Message   string   `xml:"Error>Message"`
+	RequestID string   `xml:"RequestId"`
+}
+
+type invalidChangeBatchXMLErrorResponse struct {
+	XMLName  xml.Name `xml:"InvalidChangeBatch"`
+	Messages []string `xml:"Messages>Message"`
+}
+
+func unmarshalError(r *request.Request) {
+	switch r.Operation.Name {
+	case opChangeResourceRecordSets:
+		unmarshalChangeResourceRecordSetsError(r)
+	default:
+		restxml.UnmarshalError(r)
+	}
+}
+
+func unmarshalChangeResourceRecordSetsError(r *request.Request) {
+	defer r.HTTPResponse.Body.Close()
+
+	responseBody, err := ioutil.ReadAll(r.HTTPResponse.Body)
+
+	if err != nil {
+		r.Error = awserr.New("SerializationError", "failed to read Route53 XML error response", err)
+		return
+	}
+
+	baseError := &baseXMLErrorResponse{}
+
+	if err := xml.Unmarshal(responseBody, baseError); err != nil {
+		r.Error = awserr.New("SerializationError", "failed to decode Route53 XML error response", err)
+		return
+	}
+
+	switch baseError.XMLName.Local {
+	case "InvalidChangeBatch":
+		unmarshalInvalidChangeBatchError(r, responseBody)
+	default:
+		r.HTTPResponse.Body = ioutil.NopCloser(bytes.NewReader(responseBody))
+		restxml.UnmarshalError(r)
+	}
+}
+
+func unmarshalInvalidChangeBatchError(r *request.Request, requestBody []byte) {
+	resp := &invalidChangeBatchXMLErrorResponse{}
+	err := xml.Unmarshal(requestBody, resp)
+
+	if err != nil {
+		r.Error = awserr.New("SerializationError", "failed to decode query XML error response", err)
+		return
+	}
+
+	const errorCode = "InvalidChangeBatch"
+	errors := []error{}
+
+	for _, msg := range resp.Messages {
+		errors = append(errors, awserr.New(errorCode, msg, nil))
+	}
+
+	r.Error = awserr.NewRequestFailure(
+		awserr.NewBatchError(errorCode, "ChangeBatch errors occured", errors),
+		r.HTTPResponse.StatusCode,
+		r.RequestID,
+	)
+
+}

service/route53/unmarshal_error_leak_test.go

@@ -0,0 +1,37 @@
+package route53
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/aws/aws-sdk-go/aws/request"
+	"github.com/aws/aws-sdk-go/awstesting"
+)
+
+func TestUnmarhsalErrorLeak(t *testing.T) {
+	req := &request.Request{
+		Operation: &request.Operation{
+			Name: opChangeResourceRecordSets,
+		},
+		HTTPRequest: &http.Request{
+			Header: make(http.Header),
+			Body:   &awstesting.ReadCloser{Size: 2048},
+		},
+	}
+	req.HTTPResponse = &http.Response{
+		Body: &awstesting.ReadCloser{Size: 2048},
+		Header: http.Header{
+			"X-Amzn-Requestid": []string{"1"},
+		},
+		StatusCode: http.StatusOK,
+	}
+
+	reader := req.HTTPResponse.Body.(*awstesting.ReadCloser)
+	unmarshalError(req)
+
+	assert.NotNil(t, req.Error)
+	assert.Equal(t, reader.Closed, true)
+	assert.Equal(t, reader.Size, 0)
+}

service/route53/unmarshal_error_test.go

@@ -0,0 +1,111 @@
+package route53_test
+
+import (
+	"bytes"
+	"io/ioutil"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/aws/aws-sdk-go/aws/request"
+	"github.com/aws/aws-sdk-go/awstesting/unit"
+	"github.com/aws/aws-sdk-go/service/route53"
+)
+
+func makeClientWithResponse(response string) *route53.Route53 {
+	r := route53.New(unit.Session)
+	r.Handlers.Send.Clear()
+	r.Handlers.Send.PushBack(func(r *request.Request) {
+		body := ioutil.NopCloser(bytes.NewReader([]byte(response)))
+		r.HTTPResponse = &http.Response{
+			ContentLength: int64(len(response)),
+			StatusCode:    400,
+			Status:        "Bad Request",
+			Body:          body,
+		}
+	})
+
+	return r
+}
+
+func TestUnmarshalStandardError(t *testing.T) {
+	const errorResponse = `<?xml version="1.0" encoding="UTF-8"?>
+<ErrorResponse xmlns="https://route53.amazonaws.com/doc/2013-04-01/">
+  <Error>
+    <Code>InvalidDomainName</Code>
+    <Message>The domain name is invalid</Message>
+  </Error>
+  <RequestId>12345</RequestId>
+</ErrorResponse>
+`
+
+	r := makeClientWithResponse(errorResponse)
+
+	_, err := r.CreateHostedZone(&route53.CreateHostedZoneInput{
+		CallerReference: aws.String("test"),
+		Name:            aws.String("test_zone"),
+	})
+
+	assert.Error(t, err)
+	assert.Equal(t, "InvalidDomainName", err.(awserr.Error).Code())
+	assert.Equal(t, "The domain name is invalid", err.(awserr.Error).Message())
+}
+
+func TestUnmarshalInvalidChangeBatch(t *testing.T) {
+	const errorMessage = `
+Tried to create resource record set duplicate.example.com. type A,
+but it already exists
+`
+	const errorResponse = `<?xml version="1.0" encoding="UTF-8"?>
+<InvalidChangeBatch xmlns="https://route53.amazonaws.com/doc/2013-04-01/">
+  <Messages>
+    <Message>` + errorMessage + `</Message>
+  </Messages>
+</InvalidChangeBatch>
+`
+
+	r := makeClientWithResponse(errorResponse)
+
+	req := &route53.ChangeResourceRecordSetsInput{
+		HostedZoneId: aws.String("zoneId"),
+		ChangeBatch: &route53.ChangeBatch{
+			Changes: []*route53.Change{
+				&route53.Change{
+					Action: aws.String("CREATE"),
+					ResourceRecordSet: &route53.ResourceRecordSet{
+						Name: aws.String("domain"),
+						Type: aws.String("CNAME"),
+						TTL:  aws.Int64(120),
+						ResourceRecords: []*route53.ResourceRecord{
+							{
+								Value: aws.String("cname"),
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	_, err := r.ChangeResourceRecordSets(req)
+	assert.Error(t, err)
+
+	if reqErr, ok := err.(awserr.RequestFailure); ok {
+		assert.Error(t, reqErr)
+		assert.Equal(t, 400, reqErr.StatusCode())
+	} else {
+		assert.Fail(t, "returned error is not a RequestFailure")
+	}
+
+	if batchErr, ok := err.(awserr.BatchedErrors); ok {
+		errs := batchErr.OrigErrs()
+		assert.Len(t, errs, 1)
+		assert.Equal(t, "InvalidChangeBatch", errs[0].(awserr.Error).Code())
+		assert.Equal(t, errorMessage, errs[0].(awserr.Error).Message())
+	} else {
+		assert.Fail(t, "returned error is not a BatchedErrors")
+	}
+}