Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update APIError struct to use new NullAPIErrorObject type for safety #272

Merged
merged 1 commit into from
Feb 13, 2021
Merged

Update APIError struct to use new NullAPIErrorObject type for safety #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,33 @@ 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
}

// 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
}

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.
Expand All @@ -80,12 +107,14 @@ type APIError struct {
// StatusCode is the HTTP response status code
StatusCode int `json:"-"`

// APIError represents the object returned by the API when an error occurs.
// If the response has no error object present, this will be nil.
// 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.
//
// This includes messages that should hopefully provide useful context to
// the end user.
APIError *APIErrorObject `json:"error"`
// 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 NullAPIErrorObject `json:"error"`

message string
}
Expand All @@ -97,13 +126,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,
)
}

Expand All @@ -124,7 +153,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 {
Expand Down
87 changes: 55 additions & 32 deletions client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down