PagerDuty / go-pagerduty

go client library for PagerDuty v2 API
https://v2.developer.pagerduty.com/docs/rest-api
Apache License 2.0
287 stars 241 forks source link

json: cannot unmarshal array into Go struct field APIError.error of type pagerduty.APIErrorObject #478

Closed icholy closed 10 months ago

icholy commented 1 year ago

I'm getting the following error while trying to post a change event with too many links:

failed to post change event: HTTP response with status code 400, JSON error object decode failed: json: cannot unmarshal array into Go struct field APIError.error of type pagerduty.APIErrorObject

The the response body contains:

{"error":["links should have at most 50 item(s)"]}

This fails to unmarshal to either of these structs:

https://github.com/PagerDuty/go-pagerduty/blob/b6aa0d76f875e1de499b2a246c82882f022a0f8a/client.go#L77-L95

ChuckCrawford commented 10 months ago

Hey @icholy : Do you have a use case where you actually need the structured error response? Are primarily just wanting to be able to see something about why the call failed? Or are you trying to programmatically react based on the body?

I am thinking about whether it might be more sustainable to do something like:

1) Attempt to unmarshal APIErrorObject 2) This fails, store the raw response body

icholy commented 10 months ago

@ChuckCrawford I don't need programmatic access, I just want useful error messages for debugging purposes.

I am thinking about whether it might be more sustainable to do something like:

Attempt to unmarshal APIErrorObject This fails, store the raw response body

Yeah that works for me. My PR special cases the malformed response because you're already doing that here https://github.com/PagerDuty/go-pagerduty/blob/main/client.go#L86-L94 If we take your approach I think we should also remove the linked fallback structure as well.

Where do you think the raw body should go? APIError, NullAPIErrorObject, or APIErrorObject?

ChuckCrawford commented 10 months ago

Hold off on doing anything quite yet though. Your PR might be good for now and we can revisit this if we continue to have issues. Will get back to you soon.

ChuckCrawford commented 10 months ago

Okay I am going to go ahead and merge your PR as-is for now. This provides a good place to "munge" other error formats into the default one.

We can revisit this to consider including the raw response if this continues to be a problem. In the meantime we we have the DebugCaptureLastResponse mechanism for those "other" error formats.