mailchimp-client-lib-codegen icon indicating copy to clipboard operation
mailchimp-client-lib-codegen copied to clipboard

Parse ruby error response body and add attributes

Open dhughes opened this issue 1 year ago • 4 comments

Description

This PR is based on a PR I originally opened on the generated mailchimp-marketing-ruby gem in 2021. At the time I didn't know that the gem was generated and the PR was rejected several months later. I never got around to submitting the patch to this project and have been using my own fork. However, I would like to see if my change could make it into this repo.

This PR should generally be backwards compatible, with the exception that the error message won't show a stringified version of a ruby hash.

Here's the summary from the original PR on the wrong repo:

The MailchimpMarketing::ApiError is used to wrap up error responses from the Mailchimp API. This class is used to raise errors encountered by the ApiClient class. In the case that we have a response body, the body's json is parsed into a hash and this hash is passed in as a named argument to the ApiError constructor. In the case that a hash is provided to the ApiErrors constructor (and the hash doesn't have a :message key), it is simply passed to StandardError constructor.

Because a hash is being passed to the StandardError constructor, the message value of the resulting error is the result of calling to_s on the hash. EG:

pry(#<ListsController>)> error.message
=> "{:status=>401, :response_body=>\"{\\\"type\\\":\\\"http://developer.mailchimp.com/documentation/mailchimp/guides/error-glossary/\\\",\\\"title\\\":\\\"API Key Invalid\\\",\\\"status\\\":401,\\\"detail\\\":\\\"Your API key may be invalid, or you've attempted to access the wrong datacenter.\\\",\\\"instance\\\":\\\"ff205bd4-c256-4570-aaf0-6764d0032a8a\\\"}\"}"

Note that the value of message is definitely not JSON. 😄

The ApiError class' constructor does take all of the named values provided to the constructor and store them as instance variables.

It would be nice to have access to the response body, if available. I do not feel comfortable calling eval on the hash stored in message, especially since this could potentially be a string. I do not want to use instance_variable_get to read the instance variable, since this would break encapsulation.

As such, this PR simply introduces a new attr_reader for :response_headers and :response_body. Now this data is safely accessible from outside of the error class.

As a note, there are no uses of :response_header in this gem. However, the comments in the ApiError comments show this as an example option, so I added the reader. This doesn't cover any cases where other non-standard attributes are provided to the constructor.

Known Issues

dhughes avatar Mar 25 '23 14:03 dhughes

Contributor License Agreement Instructions Thanks for your pull request. Before we can review your work, you’ll need to sign a Contributor License Agreement (CLA).

Please download the appropriate CLA below. Once downloaded, please read, sign, and send back to us at [email protected]. Please note, this account is not monitored so please visit https://mailchimp.com/contact/ if you need support.

Individual CLA: Mailchimp Individual CLA Corporate CLA: Mailchimp Corporate CLA

Once you’ve emailed us the signed CLA, please reply here (e.g. CLA signed and sent!) and we’ll verify it.

What to do if you already signed the CLA Individual signers • If you’ve previously sent us a signed CLA, please reply here letting us know and we’ll verify. If we are unable to verify, It’s possible we don’t have your GitHub username or you’re using a different email address on your Git commit. Check that the CLA you previously submitted was sent to us using the email address associated with your GitHub username and verify that your email is set on your Git commits. Corporate signers • Your company has a Point of Contact (POC) who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you’ve previously sent us an updated CLA, please reply here letting us know and we’ll verify. • The email used to register you as an authorized contributor must be the email used for the Git commit. • The email used to register you as an authorized contributor must also be attached to your GitHub account.

cla-bot[bot] avatar Mar 25 '23 14:03 cla-bot[bot]

CLA signed and sent!

dhughes avatar Mar 25 '23 14:03 dhughes

I submitted another PR earlier today and had signed and submitted the CLA at that time.

dhughes avatar Mar 25 '23 14:03 dhughes

Any update on this? Would love to see this merged.

luxflux avatar Feb 13 '24 18:02 luxflux

Same here, update on this?

jimryanzulueta avatar Oct 01 '24 10:10 jimryanzulueta