Skip to content

Commit

Permalink
Handle unexpected type changes in PagerDuty REST API error responses
Browse files Browse the repository at this point in the history
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
  • Loading branch information
theckman committed Nov 10, 2021
1 parent e0b3ce1 commit 566c7be
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 3 deletions.
35 changes: 32 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
57 changes: 57 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -680,3 +680,60 @@ 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_errors",
json: `{"code":"42","message":"test message","errors":"first error"}`,
err: "json: cannot unmarshal string into Go struct field APIErrorObject.code of type int",
},
}

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

0 comments on commit 566c7be

Please sign in to comment.