From 0933613394cf37960b82c3fc0c1b7285caae122e Mon Sep 17 00:00:00 2001
From: Tim Heckman <t@heckman.io>
Date: Tue, 9 Nov 2021 17:21:51 -0800
Subject: [PATCH] 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      | 35 +++++++++++++++++++++++++---
 client_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 3 deletions(-)

diff --git a/client.go b/client.go
index abfb849f..0c5adcf6 100644
--- a/client.go
+++ b/client.go
@@ -83,6 +83,16 @@ 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.
@@ -90,7 +100,7 @@ type APIErrorObject struct {
 // 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
diff --git a/client_test.go b/client_test.go
index f88cdb20..3df22f4a 100644
--- a/client_test.go
+++ b/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)
+		})
+	}
+}