Repository: PagerDuty/go-pagerduty
Pull request #419 (2022-01-24 10:15:49)

Return custom error type for Events API (V2) calls
This was originally opened via #368, but the original author didn't seem able to be able address the last round of feedback. So I squashed all of their commits into one, and then added a second commit to address my feedback from that PR. 

Closes #368 


Patch:
diff --git a/client.go b/client.go
index d10e387..3ab9382 100644
--- a/client.go
+++ b/client.go
@@ -44,7 +44,7 @@ const (
 )
 
 // APIObject represents generic api json response that is shared by most
-// domain object (like escalation
+// domain objects (like escalation)
 type APIObject struct {
 	ID      string `json:"id,omitempty"`
 	Type    string `json:"type,omitempty"`
@@ -107,6 +107,8 @@ type NullAPIErrorObject struct {
 	ErrorObject APIErrorObject
 }
 
+var _ json.Unmarshaler = (*NullAPIErrorObject)(nil) // assert that it satisfies the json.Unmarshaler interface.
+
 // UnmarshalJSON satisfies encoding/json.Unmarshaler
 func (n *NullAPIErrorObject) UnmarshalJSON(data []byte) error {
 	var aeo APIErrorObject
@@ -348,7 +350,7 @@ const (
 	DebugCaptureLastRequest DebugFlag = 1 << 0
 
 	// DebugCaptureLastResponse captures the last HTTP response from the API (if
-	// there was one) and makes it available via the LastAPIReponse() method.
+	// there was one) and makes it available via the LastAPIResponse() method.
 	//
 	// This may increase memory usage / GC, as we'll be making a copy of the
 	// full HTTP response body on each request and capturing it for inspection.
@@ -575,7 +577,7 @@ func (c *Client) decodeJSON(resp *http.Response, payload interface{}) error {
 
 func (c *Client) checkResponse(resp *http.Response, err error) (*http.Response, error) {
 	if err != nil {
-		return resp, fmt.Errorf("Error calling the API endpoint: %v", err)
+		return resp, fmt.Errorf("error calling the API endpoint: %v", err)
 	}
 
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
diff --git a/event_v2.go b/event_v2.go
index 1408362..6e7dda3 100644
--- a/event_v2.go
+++ b/event_v2.go
@@ -51,6 +51,125 @@ func ManageEvent(e V2Event) (*V2EventResponse, error) {
 	return ManageEventWithContext(context.Background(), e)
 }
 
+// EventsAPIV2Error represents the error response received when an Events API V2 call fails. The
+// HTTP response code is set inside of the StatusCode field, with the EventsAPIV2Error
+// field being the wrapper around the JSON error object returned from the Events API V2.
+//
+// This type also provides some helper methods like .BadRequest(), .RateLimited(),
+// and .Temporary() to help callers reason about how to handle the error.
+type EventsAPIV2Error struct {
+	// StatusCode is the HTTP response status code.
+	StatusCode int `json:"-"`
+
+	// APIError represents the object returned by the API when an error occurs,
+	// which includes messages that should hopefully provide useful context
+	// to the end user.
+	//
+	// If the API response did not contain an error object, the .Valid field of
+	// APIError will be false. If .Valid is true, the .ErrorObject field is
+	// valid and should be consulted.
+	APIError NullEventsAPIV2ErrorObject
+
+	message string
+}
+
+var _ json.Unmarshaler = (*EventsAPIV2Error)(nil) // assert that it satisfies the json.Unmarshaler interface.
+
+// Error satisfies the error interface, and should contain the StatusCode,
+// APIError.Message, APIError.ErrorObject.Status, and APIError.Errors.
+func (e EventsAPIV2Error) Error() string {
+	if len(e.message) > 0 {
+		return e.message
+	}
+
+	if !e.APIError.Valid {
+		return fmt.Sprintf("HTTP response failed with status code %d and no JSON error object was present", e.StatusCode)
+	}
+
+	if len(e.APIError.ErrorObject.Errors) == 0 {
+		return fmt.Sprintf(
+			"HTTP response failed with status code %d, status: %s, message: %s",
+			e.StatusCode, e.APIError.ErrorObject.Status, e.APIError.ErrorObject.Message,
+		)
+	}
+
+	return fmt.Sprintf(
+		"HTTP response failed with status code %d, status: %s, message: %s: %s",
+		e.StatusCode,
+		e.APIError.ErrorObject.Status,
+		e.APIError.ErrorObject.Message,
+		apiErrorsDetailString(e.APIError.ErrorObject.Errors),
+	)
+}
+
+// UnmarshalJSON satisfies encoding/json.Unmarshaler.
+func (e *EventsAPIV2Error) UnmarshalJSON(data []byte) error {
+	var eo EventsAPIV2ErrorObject
+	if err := json.Unmarshal(data, &eo); err != nil {
+		return err
+	}
+
+	e.APIError.ErrorObject = eo
+	e.APIError.Valid = true
+
+	return nil
+}
+
+// BadRequest returns whether the event request was rejected by PagerDuty as an
+// incorrect or invalid event structure.
+func (e EventsAPIV2Error) BadRequest() bool {
+	return e.StatusCode == http.StatusBadRequest
+}
+
+// RateLimited returns whether the response had a status of 429, and as such the
+// client is rate limited. The PagerDuty rate limits should reset once per
+// minute, and for the REST API they are an account-wide rate limit (not per
+// API key or IP).
+func (e EventsAPIV2Error) RateLimited() bool {
+	return e.StatusCode == http.StatusTooManyRequests
+}
+
+// APITimeout returns whether whether the response had a status of 408,
+// indicating there was a request timeout on PagerDuty's side. This error is
+// considered temporary, and so the request should be retried.
+//
+// Please note, this does not returnn true if the Go context.Context deadline
+// was exceeded when making the request.
+func (e EventsAPIV2Error) APITimeout() bool {
+	return e.StatusCode == http.StatusRequestTimeout
+}
+
+// Temporary returns whether it was a temporary error, one of which is a
+// RateLimited error.
+func (e EventsAPIV2Error) Temporary() bool {
+	return e.RateLimited() || e.APITimeout() || (e.StatusCode >= 500 && e.StatusCode < 600)
+}
+
+// NullEventsAPIV2ErrorObject is a wrapper around the EventsAPIV2ErrorObject 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.
+//
+// We assume it's possible in exceptional failure modes for error objects to be omitted by PagerDuty.
+// As such, this wrapper type provides us a way to check if the object was
+// provided while avoiding consumers accidentally missing a nil pointer check,
+// thus crashing their whole program.
+type NullEventsAPIV2ErrorObject struct {
+	Valid       bool
+	ErrorObject EventsAPIV2ErrorObject
+}
+
+// EventsAPIV2ErrorObject represents the object returned by the Events V2 API when an error
+// occurs. This includes messages that should hopefully provide useful context
+// to the end user.
+type EventsAPIV2ErrorObject struct {
+	Status  string `json:"status,omitempty"`
+	Message string `json:"message,omitempty"`
+
+	// Errors is likely to be empty, with the relevant error presented via the
+	// Status field instead.
+	Errors []string `json:"errors,omitempty"`
+}
+
 // ManageEventWithContext handles the trigger, acknowledge, and resolve methods for an event.
 func ManageEventWithContext(ctx context.Context, e V2Event) (*V2EventResponse, error) {
 	data, err := json.Marshal(e)
@@ -73,21 +192,33 @@ func ManageEventWithContext(ctx context.Context, e V2Event) (*V2EventResponse, e
 	}
 
 	defer func() { _ = resp.Body.Close() }() // explicitly discard error
-
 	if resp.StatusCode != http.StatusAccepted {
-		bytes, err := ioutil.ReadAll(resp.Body)
+		b, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			return nil, EventsAPIV2Error{
+				StatusCode: resp.StatusCode,
+				message:    fmt.Sprintf("HTTP response with status code: %d: error: %s", resp.StatusCode, err),
+			}
+		}
+		// now try to decode the response body into the error object.
+		var eae EventsAPIV2Error
+		err = json.Unmarshal(b, &eae)
 		if err != nil {
-			return nil, fmt.Errorf("HTTP Status Code: %d", resp.StatusCode)
+			eae = EventsAPIV2Error{
+				StatusCode: resp.StatusCode,
+				message:    fmt.Sprintf("HTTP response with status code: %d, JSON unmarshal object body failed: %s, body: %s", resp.StatusCode, err, string(b)),
+			}
+			return nil, eae
 		}
 
-		return nil, fmt.Errorf("HTTP Status Code: %d, Message: %s", resp.StatusCode, string(bytes))
+		eae.StatusCode = resp.StatusCode
+		return nil, eae
 	}
 
 	var eventResponse V2EventResponse
 	if err := json.NewDecoder(resp.Body).Decode(&eventResponse); err != nil {
 		return nil, err
 	}
-
 	return &eventResponse, nil
 }
 


Test patch:
diff --git a/event_v2_test.go b/event_v2_test.go
index d590959..fd27afa 100644
--- a/event_v2_test.go
+++ b/event_v2_test.go
@@ -1,7 +1,10 @@
 package pagerduty
 
 import (
+	"encoding/json"
+	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 )
 
@@ -32,3 +35,349 @@ func TestEventV2_ManageEvent(t *testing.T) {
 
 	testEqual(t, want, res)
 }
+
+func TestEventsAPIV2Error_BadRequest(t *testing.T) {
+	tests := []struct {
+		name string
+		e    EventsAPIV2Error
+		want bool
+	}{
+		{
+			name: "bad_request",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusBadRequest,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "invalid",
+						Message: "Event object is invalid",
+						Errors:  []string{"Length of 'routing_key' is incorrect (should be 32 characters)", "'event_action' is missing or blank"},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "rate_limited",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusTooManyRequests,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "throttle exceeded",
+						Message: "Requests for this service are arriving too quickly.  Please retry later.",
+						Errors:  []string{"Enhance Your Calm."},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "InternalServerError",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusInternalServerError,
+			},
+			want: false,
+		},
+		{
+			name: "ServiceUnavailable",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusServiceUnavailable,
+			},
+			want: false,
+		},
+		{
+			name: "RequestTimeout",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusRequestTimeout,
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.e.BadRequest(); got != tt.want {
+				t.Fatalf("tt.e.BadRequest() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestEventsAPIV2Error_APITimeout(t *testing.T) {
+	tests := []struct {
+		name string
+		e    EventsAPIV2Error
+		want bool
+	}{
+		{
+			name: "bad_request",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusBadRequest,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "invalid",
+						Message: "Event object is invalid",
+						Errors:  []string{"Length of 'routing_key' is incorrect (should be 32 characters)", "'event_action' is missing or blank"},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "rate_limited",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusTooManyRequests,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "throttle exceeded",
+						Message: "Requests for this service are arriving too quickly.  Please retry later.",
+						Errors:  []string{"Enhance Your Calm."},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "InternalServerError",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusInternalServerError,
+			},
+			want: false,
+		},
+		{
+			name: "ServiceUnavailable",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusServiceUnavailable,
+			},
+			want: false,
+		},
+		{
+			name: "RequestTimeout",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusRequestTimeout,
+			},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.e.APITimeout(); got != tt.want {
+				t.Fatalf("tt.e.BadRequest() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestEventsAPIV2Error_RateLimited(t *testing.T) {
+	tests := []struct {
+		name string
+		e    EventsAPIV2Error
+		want bool
+	}{
+		{
+			name: "rate_limited",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusTooManyRequests,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "throttle exceeded",
+						Message: "Requests for this service are arriving too quickly.  Please retry later.",
+						Errors:  []string{"Enhance Your Calm"},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "not_found",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusNotFound,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "Not Found",
+						Message: "Not Found",
+						Errors:  []string{"Not Found"},
+					},
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.e.RateLimited(); got != tt.want {
+				t.Fatalf("tt.e.RateLimited() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestEventsAPIV2Error_Temporary(t *testing.T) {
+	tests := []struct {
+		name string
+		e    EventsAPIV2Error
+		want bool
+	}{
+		{
+			name: "rate_limited",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusTooManyRequests,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "throttle exceeded",
+						Message: "Requests for this service are arriving too quickly.  Please retry later.",
+						Errors:  []string{"Enhance Your Calm"},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "InternalServerError",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusInternalServerError,
+			},
+			want: true,
+		},
+		{
+			name: "ServiceUnavailable",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusServiceUnavailable,
+			},
+			want: true,
+		},
+		{
+			name: "RequestTimeout",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusRequestTimeout,
+			},
+			want: true,
+		},
+		{
+			name: "not_found",
+			e: EventsAPIV2Error{
+				StatusCode: http.StatusNotFound,
+				APIError: NullEventsAPIV2ErrorObject{
+					Valid: true,
+					ErrorObject: EventsAPIV2ErrorObject{
+						Status:  "Not Found",
+						Message: "Not Found",
+						Errors:  []string{"Not Found"},
+					},
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.e.Temporary(); got != tt.want {
+				t.Fatalf("tt.e.Temporary() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestEventsAPIV2Error_Error(t *testing.T) {
+	t.Run("json_tests", func(t *testing.T) {
+		tests := []struct {
+			name  string
+			input string
+			want  string
+		}{
+			{
+				name:  "no_error",
+				input: `{"status": "Calming", "message": "Enhance Your Calm"}`,
+				want:  "HTTP response failed with status code 429, status: Calming, message: Enhance Your Calm",
+			},
+			{
+				name:  "one_error",
+				input: `{"message": "Enhance Your Calm", "status": "Calming", "errors": ["No Seriously, Enhance Your Calm"]}`,
+				want:  "HTTP response failed with status code 429, status: Calming, message: Enhance Your Calm: No Seriously, Enhance Your Calm",
+			},
+			{
+				name:  "two_error",
+				input: `{"message": "Enhance Your Calm", "status": "Calming", "errors":["No Seriously, Enhance Your Calm", "Slow Your Roll"]}`,
+				want:  "HTTP response failed with status code 429, status: Calming, message: Enhance Your Calm: No Seriously, Enhance Your Calm (and 1 more error...)",
+			},
+			{
+				name:  "three_error",
+				input: `{"message": "Enhance Your Calm", "status": "Calming", "errors":["No Seriously, Enhance Your Calm", "Slow Your Roll", "No, really..."]}`,
+				want:  "HTTP response failed with status code 429, status: Calming, message: Enhance Your Calm: No Seriously, Enhance Your Calm (and 2 more errors...)",
+			},
+		}
+
+		for _, tt := range tests {
+			t.Run(tt.name, func(t *testing.T) {
+				var a EventsAPIV2Error
+
+				if err := json.Unmarshal([]byte(tt.input), &a); err != nil {
+					t.Fatalf("failed to unmarshal JSON: %s", err)
+				}
+
+				a.StatusCode = 429
+
+				if got := a.Error(); got != tt.want {
+					t.Errorf("a.Error() = %q, want %q", got, tt.want)
+				}
+			})
+		}
+	})
+
+	tests := []struct {
+		name string
+		a    EventsAPIV2Error
+		want string
+	}{
+		{
+			name: "message",
+			a: EventsAPIV2Error{
+				message: "test message",
+			},
+			want: "test message",
+		},
+		{
+			name: "APIError_nil",
+			a: EventsAPIV2Error{
+				StatusCode: http.StatusServiceUnavailable,
+			},
+			want: "HTTP response failed with status code 503 and no JSON error object was present",
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.a.Error(); got != tt.want {
+				fmt.Println(got)
+				fmt.Println(tt.want)
+				t.Fatalf("tt.a.Error() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestEventsAPIV2Error_UnmarshalJSON(t *testing.T) {
+	v := &EventsAPIV2Error{}
+	if err := v.UnmarshalJSON([]byte(`{`)); !strings.Contains(err.Error(), "unexpected end of JSON input") {
+		t.Error("exepcted error not seen")
+	}
+}