API error responses no longer contain error code explanations

[metskem]

A couple of months ago I updated to the 1.4.0 release of the pagerduty golang library.
I noticed that error responses now have become more cryptic, and leaves us basically in the dark.
I have 2 examples with the response from 1.3.0 and 1.4.0:

1.3.0:
Failed call API endpoint. HTTP response code: 400. Error: &{2001 Invalid Input Provided [User cannot be removed as they belong to an escalation policy on this team]}
1.4.0:
HTTP response failed with status code 400, message: Invalid Input Provided (code: 2001)

1.3.0:
Failed call API endpoint. HTTP response code: 400. Error: &{2001 Invalid Input Provided [User cannot be removed as they belong to an escalation policy on this team]}
1.4.0:
HTTP response failed with status code 400, message: Invalid Input Provided (code: 2001)

So basically, both errors are summarized as "Invalid Input Provided (code: 2001)", which leaves us clueless.

For now I reverted back to 1.3.0 (we were not yet using Tag support).

Would it be possible to fix this, and give more clear error responses again?

[theckman]

@metskem I can take a look at that this week. Alternatively, if you're able to dig-in and raise a PR against the release-1.4.x branch I can review+merge it. It's a little less friction for me to merge someone else's change than to have my own merged because of repo permissions. 😄

Separately/In the mean time, are you able to use the APIError field here to get that additional context: https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#APIError

Also, v1.4.0 is only a few days old and this new API error handling was added then.

[theckman]

Actually, the v1.5.0 tree hasn't been started on master yet. This means we can raise the PR against master, and I'll make sure it gets onto the release-1.4.x branch.

[metskem]

@metskem I can take a look at that this week. Alternatively, if you're able to dig-in and raise a PR against the release-1.4.x branch I can review+merge it. It's a little less friction for me to merge someone else's change than to have my own merged because of repo permissions. 😄

Separately/In the mean time, are you able to use the APIError field here to get that additional context: https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#APIError

Also, v1.4.0 is only a few days old and this new API error handling was added then.

I can use APIError like in

		err = pd.DeleteUser("notthere")
		if err != nil {
			var aerr pagerduty.APIError
			if errors.As(err, &aerr) {
				log.Printf("aerror: %v", aerr)

But then still I don't know how to get the detailed error explanation (I might be missing something here).

[theckman]

@metskem Assuming we are inside of that errors.As() if block:

if aerr.APIError.Valid {
  fmt.Printf"top-level error message and code: %s (%d)\n", aerr.APIError.ErrorObject.Message, aerr.APIError.ErrorObject.Code)

  // get all error strings
  for i, errStr := range aerr.APIError.ErrorObject.Errors {
    fmt.Printf("error %d: %s\n", i, errStr)
  }
}

As of this moment, the error string printed does not include those additional error strings from the API. We could capture the first error string in the response, but it'd still be on consumers to pick the error apart and show the rest.

[metskem]

That works perfect Tim. Thanks for the quick response, I appreciate.
I guess we can close the issue now.

[theckman]

I raised #334, which should help add some fidelity back to the error strings. It'll be part of v1.5.0 once it's released.