Skip to content
This repository was archived by the owner on Feb 12, 2025. It is now read-only.

Commit 745c81a

Browse files
jhendrixMSFTjhendrixMSFT
authored
Handle other ServiceError corner-cases (#609)
* Handle other ServiceError corner-cases Improve unmarshalling support for errors that are not OData v4 compliant. Include raw JSON body when failing to unmarshall into ServiceError. * refine inner error string case
1 parent 9fc88b1 commit 745c81a

File tree

2 files changed

+224
-40
lines changed

2 files changed

+224
-40
lines changed

‎autorest/azure/azure.go

+69-38
Original file line numberDiff line numberDiff line change
@@ -92,54 +92,85 @@ func (se ServiceError) Error() string {
9292

9393
// UnmarshalJSON implements the json.Unmarshaler interface for the ServiceError type.
9494
func (se *ServiceError) UnmarshalJSON(b []byte) error {
95-
// per the OData v4 spec the details field must be an array of JSON objects.
96-
// unfortunately not all services adhear to the spec and just return a single
97-
// object instead of an array with one object. so we have to perform some
98-
// shenanigans to accommodate both cases.
9995
// http://docs.oasis-open.org/odata/odata-json-format/v4.0/os/odata-json-format-v4.0-os.html#_Toc372793091
10096

101-
type serviceError1 struct {
97+
type serviceErrorInternal struct {
10298
Code string `json:"code"`
10399
Message string `json:"message"`
104-
Target *string `json:"target"`
105-
Details []map[string]interface{} `json:"details"`
106-
InnerError map[string]interface{} `json:"innererror"`
107-
AdditionalInfo []map[string]interface{} `json:"additionalInfo"`
100+
Target *string `json:"target,omitempty"`
101+
AdditionalInfo []map[string]interface{} `json:"additionalInfo,omitempty"`
102+
// not all services conform to the OData v4 spec.
103+
// the following fields are where we've seen discrepancies
104+
105+
// spec calls for []map[string]interface{} but have seen map[string]interface{}
106+
Details interface{} `json:"details,omitempty"`
107+
108+
// spec calls for map[string]interface{} but have seen []map[string]interface{} and string
109+
InnerError interface{} `json:"innererror,omitempty"`
108110
}
109111

110-
type serviceError2 struct {
111-
Code string `json:"code"`
112-
Message string `json:"message"`
113-
Target *string `json:"target"`
114-
Details map[string]interface{} `json:"details"`
115-
InnerError map[string]interface{} `json:"innererror"`
116-
AdditionalInfo []map[string]interface{} `json:"additionalInfo"`
112+
sei := serviceErrorInternal{}
113+
if err := json.Unmarshal(b, &sei); err != nil {
114+
return err
117115
}
118116

119-
se1 := serviceError1{}
120-
err := json.Unmarshal(b, &se1)
121-
if err == nil {
122-
se.populate(se1.Code, se1.Message, se1.Target, se1.Details, se1.InnerError, se1.AdditionalInfo)
123-
return nil
117+
// copy the fields we know to be correct
118+
se.AdditionalInfo = sei.AdditionalInfo
119+
se.Code = sei.Code
120+
se.Message = sei.Message
121+
se.Target = sei.Target
122+
123+
// converts an []interface{} to []map[string]interface{}
124+
arrayOfObjs := func(v interface{}) ([]map[string]interface{}, bool) {
125+
arrayOf, ok := v.([]interface{})
126+
if !ok {
127+
return nil, false
128+
}
129+
final := []map[string]interface{}{}
130+
for _, item := range arrayOf {
131+
as, ok := item.(map[string]interface{})
132+
if !ok {
133+
return nil, false
134+
}
135+
final = append(final, as)
136+
}
137+
return final, true
124138
}
125139

126-
se2 := serviceError2{}
127-
err = json.Unmarshal(b, &se2)
128-
if err == nil {
129-
se.populate(se2.Code, se2.Message, se2.Target, nil, se2.InnerError, se2.AdditionalInfo)
130-
se.Details = append(se.Details, se2.Details)
131-
return nil
140+
// convert the remaining fields, falling back to raw JSON if necessary
141+
142+
if c, ok := arrayOfObjs(sei.Details); ok {
143+
se.Details = c
144+
} else if c, ok := sei.Details.(map[string]interface{}); ok {
145+
se.Details = []map[string]interface{}{c}
146+
} else if sei.Details != nil {
147+
// stuff into Details
148+
se.Details = []map[string]interface{}{
149+
{"raw": sei.Details},
150+
}
132151
}
133-
return err
134-
}
135152

136-
func (se *ServiceError) populate(code, message string, target *string, details []map[string]interface{}, inner map[string]interface{}, additional []map[string]interface{}) {
137-
se.Code = code
138-
se.Message = message
139-
se.Target = target
140-
se.Details = details
141-
se.InnerError = inner
142-
se.AdditionalInfo = additional
153+
if c, ok := sei.InnerError.(map[string]interface{}); ok {
154+
se.InnerError = c
155+
} else if c, ok := arrayOfObjs(sei.InnerError); ok {
156+
// if there's only one error extract it
157+
if len(c) == 1 {
158+
se.InnerError = c[0]
159+
} else {
160+
// multiple errors, stuff them into the value
161+
se.InnerError = map[string]interface{}{
162+
"multi": c,
163+
}
164+
}
165+
} else if c, ok := sei.InnerError.(string); ok {
166+
se.InnerError = map[string]interface{}{"error": c}
167+
} else if sei.InnerError != nil {
168+
// stuff into InnerError
169+
se.InnerError = map[string]interface{}{
170+
"raw": sei.InnerError,
171+
}
172+
}
173+
return nil
143174
}
144175

145176
// RequestError describes an error response returned by Azure service.
@@ -310,7 +341,7 @@ func WithErrorUnlessStatusCode(codes ...int) autorest.RespondDecorator {
310341
// Check if error is unwrapped ServiceError
311342
decoder := autorest.NewDecoder(encodedAs, bytes.NewReader(b.Bytes()))
312343
if err := decoder.Decode(&e.ServiceError); err != nil {
313-
return err
344+
return fmt.Errorf("autorest/azure: error response cannot be parsed: %q error: %v", b.String(), err)
314345
}
315346

316347
// for example, should the API return the literal value `null` as the response
@@ -333,7 +364,7 @@ func WithErrorUnlessStatusCode(codes ...int) autorest.RespondDecorator {
333364
rawBody := map[string]interface{}{}
334365
decoder := autorest.NewDecoder(encodedAs, bytes.NewReader(b.Bytes()))
335366
if err := decoder.Decode(&rawBody); err != nil {
336-
return err
367+
return fmt.Errorf("autorest/azure: error response cannot be parsed: %q error: %v", b.String(), err)
337368
}
338369

339370
e.ServiceError = &ServiceError{

‎autorest/azure/azure_test.go

+155-2
Original file line numberDiff line numberDiff line change
@@ -531,11 +531,14 @@ func TestRequestErrorString_WithError(t *testing.T) {
531531
}
532532

533533
func TestRequestErrorString_WithErrorNonConforming(t *testing.T) {
534+
// here details is an object, it should be an array of objects
535+
// and innererror is an array of objects (should be one object)
534536
j := `{
535537
"error": {
536538
"code": "InternalError",
537539
"message": "Conflict",
538-
"details": {"code": "conflict1", "message":"error message1"}
540+
"details": {"code": "conflict1", "message":"error message1"},
541+
"innererror": [{ "customKey": "customValue" }]
539542
}
540543
}`
541544
uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
@@ -553,12 +556,162 @@ func TestRequestErrorString_WithErrorNonConforming(t *testing.T) {
553556
t.Fatalf("azure: returned nil error for proper error response")
554557
}
555558
azErr, _ := err.(*RequestError)
556-
expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"code\":\"conflict1\",\"message\":\"error message1\"}]"
559+
expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"code\":\"conflict1\",\"message\":\"error message1\"}] InnerError={\"customKey\":\"customValue\"}"
557560
if expected != azErr.Error() {
558561
t.Fatalf("azure: send wrong RequestError.\nexpected=%v\ngot=%v", expected, azErr.Error())
559562
}
560563
}
561564

565+
func TestRequestErrorString_WithErrorNonConforming2(t *testing.T) {
566+
// here innererror is a string (it should be a JSON object)
567+
j := `{
568+
"error": {
569+
"code": "InternalError",
570+
"message": "Conflict",
571+
"details": {"code": "conflict1", "message":"error message1"},
572+
"innererror": "something bad happened"
573+
}
574+
}`
575+
uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
576+
r := mocks.NewResponseWithContent(j)
577+
mocks.SetResponseHeader(r, HeaderRequestID, uuid)
578+
r.Request = mocks.NewRequest()
579+
r.StatusCode = http.StatusInternalServerError
580+
r.Status = http.StatusText(r.StatusCode)
581+
582+
err := autorest.Respond(r,
583+
WithErrorUnlessStatusCode(http.StatusOK),
584+
autorest.ByClosing())
585+
586+
if err == nil {
587+
t.Fatalf("azure: returned nil error for proper error response")
588+
}
589+
azErr, _ := err.(*RequestError)
590+
expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"code\":\"conflict1\",\"message\":\"error message1\"}] InnerError={\"error\":\"something bad happened\"}"
591+
if expected != azErr.Error() {
592+
t.Fatalf("azure: send wrong RequestError.\nexpected=%v\ngot=%v", expected, azErr.Error())
593+
}
594+
}
595+
596+
func TestRequestErrorString_WithErrorNonConforming3(t *testing.T) {
597+
// here details is an object, it should be an array of objects
598+
// and innererror is an array of objects (should be one object)
599+
j := `{
600+
"error": {
601+
"code": "InternalError",
602+
"message": "Conflict",
603+
"details": {"code": "conflict1", "message":"error message1"},
604+
"innererror": [{ "customKey": "customValue" }, { "customKey2": "customValue2" }]
605+
}
606+
}`
607+
uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
608+
r := mocks.NewResponseWithContent(j)
609+
mocks.SetResponseHeader(r, HeaderRequestID, uuid)
610+
r.Request = mocks.NewRequest()
611+
r.StatusCode = http.StatusInternalServerError
612+
r.Status = http.StatusText(r.StatusCode)
613+
614+
err := autorest.Respond(r,
615+
WithErrorUnlessStatusCode(http.StatusOK),
616+
autorest.ByClosing())
617+
618+
if err == nil {
619+
t.Fatalf("azure: returned nil error for proper error response")
620+
}
621+
azErr, _ := err.(*RequestError)
622+
expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"code\":\"conflict1\",\"message\":\"error message1\"}] InnerError={\"multi\":[{\"customKey\":\"customValue\"},{\"customKey2\":\"customValue2\"}]}"
623+
if expected != azErr.Error() {
624+
t.Fatalf("azure: send wrong RequestError.\nexpected=%v\ngot=%v", expected, azErr.Error())
625+
}
626+
}
627+
628+
func TestRequestErrorString_WithErrorNonConforming4(t *testing.T) {
629+
// here details is a string, it should be an array of objects
630+
j := `{
631+
"error": {
632+
"code": "InternalError",
633+
"message": "Conflict",
634+
"details": "something bad happened"
635+
}
636+
}`
637+
uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
638+
r := mocks.NewResponseWithContent(j)
639+
mocks.SetResponseHeader(r, HeaderRequestID, uuid)
640+
r.Request = mocks.NewRequest()
641+
r.StatusCode = http.StatusInternalServerError
642+
r.Status = http.StatusText(r.StatusCode)
643+
644+
err := autorest.Respond(r,
645+
WithErrorUnlessStatusCode(http.StatusOK),
646+
autorest.ByClosing())
647+
648+
if err == nil {
649+
t.Fatalf("azure: returned nil error for proper error response")
650+
}
651+
azErr, _ := err.(*RequestError)
652+
expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"raw\":\"something bad happened\"}]"
653+
if expected != azErr.Error() {
654+
t.Fatalf("azure: send wrong RequestError.\nexpected=%v\ngot=%v", expected, azErr.Error())
655+
}
656+
}
657+
658+
func TestRequestErrorString_WithErrorNonConforming5(t *testing.T) {
659+
// here innererror is a number (it should be a JSON object)
660+
j := `{
661+
"error": {
662+
"code": "InternalError",
663+
"message": "Conflict",
664+
"details": {"code": "conflict1", "message":"error message1"},
665+
"innererror": 500
666+
}
667+
}`
668+
uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
669+
r := mocks.NewResponseWithContent(j)
670+
mocks.SetResponseHeader(r, HeaderRequestID, uuid)
671+
r.Request = mocks.NewRequest()
672+
r.StatusCode = http.StatusInternalServerError
673+
r.Status = http.StatusText(r.StatusCode)
674+
675+
err := autorest.Respond(r,
676+
WithErrorUnlessStatusCode(http.StatusOK),
677+
autorest.ByClosing())
678+
679+
if err == nil {
680+
t.Fatalf("azure: returned nil error for proper error response")
681+
}
682+
azErr, _ := err.(*RequestError)
683+
expected := "autorest/azure: Service returned an error. Status=500 Code=\"InternalError\" Message=\"Conflict\" Details=[{\"code\":\"conflict1\",\"message\":\"error message1\"}] InnerError={\"raw\":500}"
684+
if expected != azErr.Error() {
685+
t.Fatalf("azure: send wrong RequestError.\nexpected=%v\ngot=%v", expected, azErr.Error())
686+
}
687+
}
688+
689+
func TestRequestErrorString_WithErrorNonConforming6(t *testing.T) {
690+
// here code is a number, it should be a string
691+
j := `{
692+
"code": 409,
693+
"message": "Conflict",
694+
"details": "something bad happened"
695+
}`
696+
uuid := "71FDB9F4-5E49-4C12-B266-DE7B4FD999A6"
697+
r := mocks.NewResponseWithContent(j)
698+
mocks.SetResponseHeader(r, HeaderRequestID, uuid)
699+
r.Request = mocks.NewRequest()
700+
r.StatusCode = http.StatusInternalServerError
701+
r.Status = http.StatusText(r.StatusCode)
702+
703+
err := autorest.Respond(r,
704+
WithErrorUnlessStatusCode(http.StatusOK),
705+
autorest.ByClosing())
706+
707+
if err == nil {
708+
t.Fatalf("azure: returned nil error for proper error response")
709+
}
710+
if _, ok := err.(*RequestError); ok {
711+
t.Fatal("unexpected RequestError when unmarshalling fails")
712+
}
713+
}
714+
562715
func TestParseResourceID_WithValidBasicResourceID(t *testing.T) {
563716

564717
basicResourceID := "/subscriptions/subid-3-3-4/resourceGroups/regGroupVladdb/providers/Microsoft.Network/LoadBalancer/testResourceName"

0 commit comments

Comments
 (0)