Handle unexpected type changes in PagerDuty REST API error responses

The PagerDuty REST API documentation[1] indicates that the `errors` field of an
error response is an array of error strings. However, as shown in #339 the API
sometimes violates that specification and instead returns the `errors` field as
a string.

There is a PagerDuty customer support ticket open[2] to to address this issue, but
there is currently no ETA on the resolution. As such we are going to create this
workaround to properly parse the invalid responses and return the proper error
type to callers.

Closes #339

[1] https://developer.pagerduty.com/docs/ZG9jOjExMDI5NTYz-errors
[2] https://tickets.pagerduty.com/hc/requests/341221

client.go

@@ -83,14 +83,24 @@ type APIErrorObject struct {
 	Errors  []string `json:"errors,omitempty"`
 }
 
+// fallbackAPIErrorObject is a shim to solve this issue:
+// https://github.com/PagerDuty/go-pagerduty/issues/339
+//
+// TODO: remove when PagerDuty engineering confirms bugfix to the REST API
+type fallbackAPIErrorObject struct {
+	Code    int    `json:"code,omitempty"`
+	Message string `json:"message,omitempty"`
+	Errors  string `json:"errors,omitempty"`
+}
+
 // NullAPIErrorObject is a wrapper around the APIErrorObject type. If the Valid
 // field is true, the API response included a structured error JSON object. This
 // structured object is then set on the ErrorObject field.
 //
 // While the PagerDuty REST API is documented to always return the error object,
 // we assume it's possible in exceptional failure modes for this to be omitted.
 // As such, this wrapper type provides us a way to check if the object was
-// provided while avoiding cosnumers accidentally missing a nil pointer check,
+// provided while avoiding consumers accidentally missing a nil pointer check,
 // thus crashing their whole program.
 type NullAPIErrorObject struct {
 	Valid       bool
@@ -100,8 +110,27 @@ type NullAPIErrorObject struct {
 // UnmarshalJSON satisfies encoding/json.Unmarshaler
 func (n *NullAPIErrorObject) UnmarshalJSON(data []byte) error {
 	var aeo APIErrorObject
-	if err := json.Unmarshal(data, &aeo); err != nil {
-		return err
+
+	err := json.Unmarshal(data, &aeo)
+	if err != nil {
+		terr, ok := err.(*json.UnmarshalTypeError)
+		if !ok {
+			return err
+		}
+
+		//
+		// see https://github.com/PagerDuty/go-pagerduty/issues/339
+		//
+		var faeo fallbackAPIErrorObject
+
+		if err := json.Unmarshal(data, &faeo); err != nil {
+			// still failed, so return the original error
+			return terr
+		}
+
+		aeo.Code = faeo.Code
+		aeo.Message = faeo.Message
+		aeo.Errors = []string{faeo.Errors}
 	}
 
 	n.ErrorObject = aeo

client_test.go

@@ -680,3 +680,65 @@ func TestClient_Do(t *testing.T) {
 		})
 	}
 }
+
+func TestNullAPIErrorObject_UnmarshalJSON(t *testing.T) {
+	tests := []struct {
+		name string
+		json string
+		err  string
+		want *NullAPIErrorObject
+	}{
+		{
+			name: "error_per_api_spec",
+			json: `{"code":42,"message":"test message","errors":["first error","second error"]}`,
+			want: &NullAPIErrorObject{
+				Valid: true,
+				ErrorObject: APIErrorObject{
+					Code:    42,
+					Message: "test message",
+					Errors: []string{
+						"first error",
+						"second error",
+					},
+				},
+			},
+		},
+		{
+			name: "issue_339",
+			json: `{"code":84,"message":"other message","errors":"only error"}`,
+			want: &NullAPIErrorObject{
+				Valid: true,
+				ErrorObject: APIErrorObject{
+					Code:    84,
+					Message: "other message",
+					Errors: []string{
+						"only error",
+					},
+				},
+			},
+		},
+		{
+			name: "returns_type_errors",
+			json: `{"code":"42","message":"test message","errors":"first error"}`,
+			err:  "json: cannot unmarshal string into Go struct field APIErrorObject.code of type int",
+		},
+		{
+			name: "returns_syntax_errors",
+			json: `}`,
+			err:  "invalid character '}' looking for beginning of value",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := &NullAPIErrorObject{}
+
+			err := json.Unmarshal([]byte(tt.json), &got)
+			if !testErrCheck(t, "*NullAPIErrorObject.UnmarshalJSON()", tt.err, err) {
+				return
+			}
+
+			testEqual(t, tt.want, got)
+		})
+	}
+}