go-resty / resty

Simple HTTP and REST client library for Go
MIT License
10.13k stars 710 forks source link

Support Error marshalling using application/problem+json content type #134

Closed dahankzter closed 6 years ago

dahankzter commented 6 years ago

I just found out that Mailchimp uses this header which is unfortunate and unusual afaik but there is a spec: https://tools.ietf.org/html/rfc7807 that seems to define this.

It would be nice to be able to use the SetError() functionality.

jeevatkm commented 6 years ago

@dahankzter I have read the Spec. It seems resty needs knowledge to understand it.

Let me add a support, once its ready I will ping you.

jeevatkm commented 6 years ago

@dahankzter Just thought to clarify. I'm planning to add support for application/problem+json and application/problem+xml per RFC7807

However resty only considers response status code > 399 as an error.

jeevatkm commented 6 years ago

@dahankzter I have implemented it, available on branch add-support-for-rfc7807

Please give it try and let me know.

dahankzter commented 6 years ago

I will test it as soon as possible. Having some dep problems now that go.uber.org is gone.

jeevatkm commented 6 years ago

No problem, take your time. What happened to go.uber.org?

dahankzter commented 6 years ago

Some DNS issue over at Cloudflare is the current bet.

dahankzter commented 6 years ago

The regex doesn't' seem to match application/problem+json afaik. I tried here https://regex101.com/ as well a little test.

Your test case seems to only assert that the Error is set which it is but not actually parsed and used but maybe I am wrong?

dahankzter commented 6 years ago

Making the "problem+" a group seems to work (?i:[application|text]/(problem\+)?json)

jeevatkm commented 6 years ago

@dahankzter Thank you, I have updated the regex and test case. Now it should be good.

dahankzter commented 6 years ago

Excellent! What is your release policy? When can this go out? I use it in a real project and we decided to avoid specific commits and go with proper tags as given by the respective dependency.

jeevatkm commented 6 years ago

@dahankzter Typically bug fixes gets immediately released. Feature and enhancements are monthly.

dahankzter commented 6 years ago

Ok great, I look forward to the next release! :)

jeevatkm commented 6 years ago

Merged to master :smile:

jeevatkm commented 6 years ago

@dahankzter FYI resty v1.4 released.