update azure error for details

tareksha · Jun 24, 2016 · 1b029d0 · 1b029d0
1 parent 8a8967c
commit 1b029d0
Show file tree

Hide file tree

Showing 4 changed files with 181 additions and 20 deletions.
diff --git a/autorest/azure/async_test.go b/autorest/azure/async_test.go
@@ -6,6 +6,7 @@ import (
 	"github.com/Azure/go-autorest/autorest/mocks"
 	"io/ioutil"
 	"net/http"
+	"reflect"
 	"strings"
 	"sync"
 	"testing"
@@ -936,9 +937,17 @@ func TestDoPollForAsynchronous_ReturnsErrorForFirstPutRequest(t *testing.T) {
 		t.Fatalf("azure: returned error is not azure.RequestError: %T", err)
 	}
 
-	if expected := makeRequestErrorString("InvalidParameter",
-		"tom-service-DISCOVERY-server-base-v1.core.local' is not a valid captured VHD blob name prefix.", 400); reqError.Error() != expected {
-		t.Fatalf("azure: wrong error. expected=%q; got=%q", expected, reqError)
+	expected := &RequestError{
+		ServiceError: &ServiceError{
+			Code:    "InvalidParameter",
+			Message: "tom-service-DISCOVERY-server-base-v1.core.local' is not a valid captured VHD blob name prefix.",
+		},
+		DetailedError: autorest.DetailedError{
+			StatusCode: 400,
+		},
+	}
+	if !reflect.DeepEqual(reqError, expected) {
+		t.Fatalf("azure: wrong error. expected=%q\ngot=%q", expected, reqError)
 	}
 
 	defer res.Body.Close()
@@ -1104,8 +1113,3 @@ func newProvisioningStatusResponse(status string) *http.Response {
 func makeLongRunningOperationErrorString(code string, message string) string {
 	return fmt.Sprintf("Long running operation terminated with status 'Failed': Code=%q Message=%q", code, message)
 }
-
-func makeRequestErrorString(code, message string, status int) string {
-	return fmt.Sprintf("azure: Service returned an error. Code=%q Message=%q Status=%d",
-		code, message, status)
-}
diff --git a/autorest/azure/azure.go b/autorest/azure/azure.go
@@ -6,6 +6,7 @@ See the included examples for more detail.
 package azure
 
 import (
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -29,12 +30,20 @@ const (
 
 // ServiceError encapsulates the error response from an Azure service.
 type ServiceError struct {
-	Code    string `json:"code"`
-	Message string `json:"message"`
+	Code    string         `json:"code"`
+	Message string         `json:"message"`
+	Details *[]interface{} `json:"details"`
 }
 
 func (se ServiceError) Error() string {
-	return fmt.Sprintf("Azure Error: Code=%q Message=%q", se.Code, se.Message)
+	if se.Details != nil {
+		d, err := json.Marshal(*(se.Details))
+		if err != nil {
+			return fmt.Sprintf("Code=%q Message=%q Details=%v", se.Code, se.Message, *se.Details)
+		}
+		return fmt.Sprintf("Code=%q Message=%q Details=%v", se.Code, se.Message, string(d))
+	}
+	return fmt.Sprintf("Code=%q Message=%q", se.Code, se.Message)
 }
 
 // RequestError describes an error response returned by Azure service.
@@ -50,8 +59,8 @@ type RequestError struct {
 
 // Error returns a human-friendly error message from service error.
 func (e RequestError) Error() string {
-	return fmt.Sprintf("azure: Service returned an error. Code=%q Message=%q Status=%d",
-		e.ServiceError.Code, e.ServiceError.Message, e.StatusCode)
+	return fmt.Sprintf("autorest/azure: Service returned an error. Status=%v %v",
+		e.StatusCode, e.ServiceError)
 }
 
 // IsAzureError returns true if the passed error is an Azure Service error; false otherwise.
@@ -149,14 +158,20 @@ func WithErrorUnlessStatusCode(codes ...int) autorest.RespondDecorator {
 				var e RequestError
 				defer resp.Body.Close()
 
+				// Copy and replace the Body in case it does not contain an error object.
+				// This will leave the Body available to the caller.
 				b, decodeErr := autorest.CopyAndDecode(autorest.EncodedAsJSON, resp.Body, &e)
-				resp.Body = ioutil.NopCloser(&b) // replace body with in-memory reader
-				if decodeErr != nil || e.ServiceError == nil {
-					return fmt.Errorf("autorest/azure: error response cannot be parsed: %q error: %v", b.String(), err)
+				resp.Body = ioutil.NopCloser(&b)
+				if decodeErr != nil {
+					return fmt.Errorf("autorest/azure: error response cannot be parsed: %q error: %v", b.String(), decodeErr)
+				} else if e.ServiceError == nil {
+					e.ServiceError = &ServiceError{Code: "Unknown", Message: "Unknown service error"}
 				}
 
 				e.RequestID = ExtractRequestID(resp)
-				e.StatusCode = resp.StatusCode
+				if e.StatusCode == nil {
+					e.StatusCode = resp.StatusCode
+				}
 				err = &e
 			}
 			return err

diff --git a/autorest/azure/azure_test.go b/autorest/azure/azure_test.go
@@ -1,6 +1,7 @@
 package azure
 
 import (
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -168,7 +169,7 @@ func TestWithErrorUnlessStatusCode_NotAnAzureError(t *testing.T) {
 	}
 }
 
-func TestWithErrorUnlessStatusCode_FoundAzureError(t *testing.T) {
+func TestWithErrorUnlessStatusCode_FoundAzureErrorWithoutDetails(t *testing.T) {
 	j := `{
 		"error": {
 			"code": "InternalError",
@@ -193,14 +194,126 @@ func TestWithErrorUnlessStatusCode_FoundAzureError(t *testing.T) {
 	if !ok {
 		t.Fatalf("azure: returned error is not azure.RequestError: %T", err)
 	}
+
+	expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Azure is having trouble right now.\""
+	if !reflect.DeepEqual(expected, azErr.Error()) {
+		t.Fatalf("azure: service error is not unmarshaled properly.\nexpected=%v\ngot=%v", expected, azErr.Error())
+	}
+
+	if expected := http.StatusInternalServerError; azErr.StatusCode != expected {
+		t.Fatalf("azure: got wrong StatusCode=%d Expected=%d", azErr.StatusCode, expected)
+	}
+	if expected := uuid; azErr.RequestID != expected {
+		t.Fatalf("azure: wrong request ID in error. expected=%q; got=%q", expected, azErr.RequestID)
+	}
+
+	_ = azErr.Error()
+
+	// the error body should still be there
+	defer r.Body.Close()
+	b, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(b) != j {
+		t.Fatalf("response body is wrong. got=%q expected=%q", string(b), j)
+	}
+
+}
+
+func TestWithErrorUnlessStatusCode_FoundAzureErrorWithDetails(t *testing.T) {
+	j := `{
+		"error": {
+			"code": "InternalError",
+			"message": "Azure is having trouble right now.",
+			"details": [{"code": "conflict1", "message":"error message1"}, 
+						{"code": "conflict2", "message":"error message2"}]
+		}
+	}`
+	uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
+	r := mocks.NewResponseWithContent(j)
+	mocks.SetResponseHeader(r, HeaderRequestID, uuid)
+	r.Request = mocks.NewRequest()
+	r.StatusCode = http.StatusInternalServerError
+	r.Status = http.StatusText(r.StatusCode)
+
+	err := autorest.Respond(r,
+		WithErrorUnlessStatusCode(http.StatusOK),
+		autorest.ByClosing())
+
+	if err == nil {
+		t.Fatalf("azure: returned nil error for proper error response")
+	}
+	azErr, ok := err.(*RequestError)
+	if !ok {
+		t.Fatalf("azure: returned error is not azure.RequestError: %T", err)
+	}
+
 	if expected := "InternalError"; azErr.ServiceError.Code != expected {
 		t.Fatalf("azure: wrong error code. expected=%q; got=%q", expected, azErr.ServiceError.Code)
 	}
 	if azErr.ServiceError.Message == "" {
 		t.Fatalf("azure: error message is not unmarshaled properly")
 	}
+	b, _ := json.Marshal(*azErr.ServiceError.Details)
+	if string(b) != `[{"code":"conflict1","message":"error message1"},{"code":"conflict2","message":"error message2"}]` {
+		t.Fatalf("azure: error details is not unmarshaled properly")
+	}
+
 	if expected := http.StatusInternalServerError; azErr.StatusCode != expected {
-		t.Fatalf("azure: got wrong StatusCode=%d Expected=%d", azErr.StatusCode, expected)
+		t.Fatalf("azure: got wrong StatusCode=%v Expected=%d", azErr.StatusCode, expected)
+	}
+	if expected := uuid; azErr.RequestID != expected {
+		t.Fatalf("azure: wrong request ID in error. expected=%q; got=%q", expected, azErr.RequestID)
+	}
+
+	_ = azErr.Error()
+
+	// the error body should still be there
+	defer r.Body.Close()
+	b, err = ioutil.ReadAll(r.Body)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(b) != j {
+		t.Fatalf("response body is wrong. got=%q expected=%q", string(b), j)
+	}
+
+}
+
+func TestWithErrorUnlessStatusCode_NoAzureError(t *testing.T) {
+	j := `{
+			"Status":"NotFound"
+		}`
+	uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
+	r := mocks.NewResponseWithContent(j)
+	mocks.SetResponseHeader(r, HeaderRequestID, uuid)
+	r.Request = mocks.NewRequest()
+	r.StatusCode = http.StatusInternalServerError
+	r.Status = http.StatusText(r.StatusCode)
+
+	err := autorest.Respond(r,
+		WithErrorUnlessStatusCode(http.StatusOK),
+		autorest.ByClosing())
+	if err == nil {
+		t.Fatalf("azure: returned nil error for proper error response")
+	}
+	azErr, ok := err.(*RequestError)
+	if !ok {
+		t.Fatalf("azure: returned error is not azure.RequestError: %T", err)
+	}
+
+	expected := &ServiceError{
+		Code:    "Unknown",
+		Message: "Unknown service error",
+	}
+
+	if !reflect.DeepEqual(expected, azErr.ServiceError) {
+		t.Fatalf("azure: service error is not unmarshaled properly. expected=%q\ngot=%q", expected, azErr.ServiceError)
+	}
+
+	if expected := http.StatusInternalServerError; azErr.StatusCode != expected {
+		t.Fatalf("azure: got wrong StatusCode=%v Expected=%d", azErr.StatusCode, expected)
 	}
 	if expected := uuid; azErr.RequestID != expected {
 		t.Fatalf("azure: wrong request ID in error. expected=%q; got=%q", expected, azErr.RequestID)
@@ -220,6 +333,35 @@ func TestWithErrorUnlessStatusCode_FoundAzureError(t *testing.T) {
 
 }
 
+func TestRequestErrorString_WithError(t *testing.T) {
+	j := `{
+		"error": {
+			"code": "InternalError",
+			"message": "Conflict",
+			"details": [{"code": "conflict1", "message":"error message1"}]
+		}
+	}`
+	uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
+	r := mocks.NewResponseWithContent(j)
+	mocks.SetResponseHeader(r, HeaderRequestID, uuid)
+	r.Request = mocks.NewRequest()
+	r.StatusCode = http.StatusInternalServerError
+	r.Status = http.StatusText(r.StatusCode)
+
+	err := autorest.Respond(r,
+		WithErrorUnlessStatusCode(http.StatusOK),
+		autorest.ByClosing())
+
+	if err == nil {
+		t.Fatalf("azure: returned nil error for proper error response")
+	}
+	azErr, _ := err.(*RequestError)
+	expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"code\":\"conflict1\",\"message\":\"error message1\"}]"
+	if expected != azErr.Error() {
+		t.Fatalf("azure: send wrong RequestError.\nexpected=%v\ngot=%v", expected, azErr.Error())
+	}
+}
+
 func withErrorPrepareDecorator(e *error) autorest.PrepareDecorator {
 	return func(p autorest.Preparer) autorest.Preparer {
 		return autorest.PreparerFunc(func(r *http.Request) (*http.Request, error) {

diff --git a/autorest/error.go b/autorest/error.go
@@ -24,7 +24,7 @@ type DetailedError struct {
 	Method string
 
 	// StatusCode is the HTTP Response StatusCode (if non-zero) that led to the error.
-	StatusCode int
+	StatusCode interface{}
 
 	// Message is the error message.
 	Message string