diff --git a/client.go b/client.go index 0eed9b4b..c9de68e0 100644 --- a/client.go +++ b/client.go @@ -67,6 +67,32 @@ type APIErrorObject struct { 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, +// thus crashing their whole program. +type NullAPIErrorObject struct { + Valid bool + ErrorObject APIErrorObject +} + +func (n *NullAPIErrorObject) UnmarshalJSON(data []byte) error { + var aeo APIErrorObject + if err := json.Unmarshal(data, &aeo); err != nil { + return err + } + + n.ErrorObject = aeo + n.Valid = true + + return nil +} + // APIError represents the error response received when an API call fails. The // HTTP response code is set inside of the StatusCode field, with the APIError // field being the structured JSON error object returned from the API. @@ -85,7 +111,7 @@ type APIError struct { // // This includes messages that should hopefully provide useful context to // the end user. - APIError *APIErrorObject `json:"error"` + APIError NullAPIErrorObject `json:"error"` message string } @@ -97,13 +123,13 @@ func (a APIError) Error() string { return a.message } - if a.APIError == nil { + if !a.APIError.Valid { return fmt.Sprintf("HTTP response failed with status code %d and no JSON error object was present", a.StatusCode) } return fmt.Sprintf( "HTTP response failed with status code %d, message: %s (code: %d)", - a.StatusCode, a.APIError.Message, a.APIError.Code, + a.StatusCode, a.APIError.ErrorObject.Message, a.APIError.ErrorObject.Code, ) } @@ -124,7 +150,7 @@ func (a APIError) Temporary() bool { // NotFound returns whether this was an error where it seems like the resource // was not found. func (a APIError) NotFound() bool { - return a.StatusCode == http.StatusNotFound || a.APIError.Code == 2100 + return a.StatusCode == http.StatusNotFound || (a.APIError.Valid && a.APIError.ErrorObject.Code == 2100) } func newDefaultHTTPClient() *http.Client { diff --git a/client_test.go b/client_test.go index c1f01a9c..9c5d8cbe 100644 --- a/client_test.go +++ b/client_test.go @@ -104,10 +104,13 @@ func TestAPIError_RateLimited(t *testing.T) { name: "rate_limited", a: APIError{ StatusCode: http.StatusTooManyRequests, - APIError: &APIErrorObject{ - Code: 420, - Message: "Enhance Your Calm", - Errors: []string{"Enhance Your Calm"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 420, + Message: "Enhance Your Calm", + Errors: []string{"Enhance Your Calm"}, + }, }, }, want: true, @@ -116,10 +119,13 @@ func TestAPIError_RateLimited(t *testing.T) { name: "not_found", a: APIError{ StatusCode: http.StatusNotFound, - APIError: &APIErrorObject{ - Code: 2100, - Message: "Not Found", - Errors: []string{"Not Found"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 2100, + Message: "Not Found", + Errors: []string{"Not Found"}, + }, }, }, want: false, @@ -147,10 +153,13 @@ func TestAPIError_Temporary(t *testing.T) { name: "rate_limited", a: APIError{ StatusCode: http.StatusTooManyRequests, - APIError: &APIErrorObject{ - Code: 420, - Message: "Enhance Your Calm", - Errors: []string{"Enhance Your Calm"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 420, + Message: "Enhance Your Calm", + Errors: []string{"Enhance Your Calm"}, + }, }, }, want: true, @@ -159,10 +168,13 @@ func TestAPIError_Temporary(t *testing.T) { name: "not_found", a: APIError{ StatusCode: http.StatusNotFound, - APIError: &APIErrorObject{ - Code: 2100, - Message: "Not Found", - Errors: []string{"Not Found"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 2100, + Message: "Not Found", + Errors: []string{"Not Found"}, + }, }, }, want: false, @@ -204,10 +216,13 @@ func TestAPIError_NotFound(t *testing.T) { name: "rate_limited", a: APIError{ StatusCode: http.StatusTooManyRequests, - APIError: &APIErrorObject{ - Code: 420, - Message: "Enhance Your Calm", - Errors: []string{"Enhance Your Calm"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 420, + Message: "Enhance Your Calm", + Errors: []string{"Enhance Your Calm"}, + }, }, }, want: false, @@ -216,10 +231,13 @@ func TestAPIError_NotFound(t *testing.T) { name: "not_found", a: APIError{ StatusCode: http.StatusNotFound, - APIError: &APIErrorObject{ - Code: 2100, - Message: "Not Found", - Errors: []string{"Not Found"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 2100, + Message: "Not Found", + Errors: []string{"Not Found"}, + }, }, }, want: true, @@ -228,10 +246,13 @@ func TestAPIError_NotFound(t *testing.T) { name: "not_found_weird_status", a: APIError{ StatusCode: http.StatusBadRequest, - APIError: &APIErrorObject{ - Code: 2100, - Message: "Not Found", - Errors: []string{"Not Found"}, + APIError: NullAPIErrorObject{ + Valid: true, + ErrorObject: APIErrorObject{ + Code: 2100, + Message: "Not Found", + Errors: []string{"Not Found"}, + }, }, }, want: true, @@ -240,10 +261,12 @@ func TestAPIError_NotFound(t *testing.T) { name: "not_found_weird_error_code", a: APIError{ StatusCode: http.StatusNotFound, - APIError: &APIErrorObject{ - Code: 2101, - Message: "Not Found", - Errors: []string{"Not Found"}, + APIError: NullAPIErrorObject{ + ErrorObject: APIErrorObject{ + Code: 2101, + Message: "Not Found", + Errors: []string{"Not Found"}, + }, }, }, want: true,