Handle unexpected type changes in PagerDuty REST API error responses

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)
+		})
+	}
+}