Pass on specific error message returned from Mailgun#71
Open
TylerRick wants to merge 1 commit intojorgemanrubia:masterfrom
Open
Pass on specific error message returned from Mailgun#71TylerRick wants to merge 1 commit intojorgemanrubia:masterfrom
TylerRick wants to merge 1 commit intojorgemanrubia:masterfrom
Conversation
Contributor
Author
|
Since a picture is worth a 800 words, here's what the error looked like before:
and here's how the same error shows up now:
|
When the Mailgun API returns a "400 Bad Request" error, it includes the specific error message (for example, "'to' parameter is missing") from in the JSON response. However, there was no easy way to see what that specific error message actually was, and you'd end up with a generic (and therefore useless) RestClient::BadRequest error from your Rails app that you had no idea the actual cause of. This change causes it to automatically extract that specific message from the JSON response and use that as the Ruby exception message.
TylerRick
force-pushed
the
pass_on_specific_mailgun_error_message
branch
from
054c35c to
b7338ad
Compare
Contributor
Author
|
Does this look okay? Anything I need to do to get it merged in? |
zzet
reviewed
Oct 20, 2016
| response = mailgun_client.send_message build_mailgun_message_for(rails_message) | ||
| rescue RestClient::Exception => mailgun_error | ||
| json = JSON.parse(mailgun_error.http_body.to_s) rescue raise(mailgun_error) | ||
| mailgun_error.define_singleton_method(:message) { json["message"] } |
Collaborator
There was a problem hiding this comment.
I think that bad idea to monkey patch RestClient::* classes on the fly. It is foreign classes and this changes can be counterintuitive for devs, who use API of RestClient and they can wait documented behaviour.
For this case better to use own Exception classes.
In additional case I think about using of API from mailgun-ruby gem into this gem (mailgun-ruby gem already have implemented all required api calls and no reason why we should duplicate this logic).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the Mailgun API returns a "400 Bad Request" error, it includes the specific error message (for example, "'to' parameter is missing") from in the JSON response.
However, there was no easy way to see what that specific error message actually was, and you'd end
up with a generic (and therefore useless) RestClient::BadRequest error from your Rails app that you had no idea the actual cause of.
This change causes it to automatically extract that specific message from the JSON response and use
that as the Ruby exception message.