-
Notifications
You must be signed in to change notification settings - Fork 311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Why is Github::Error::GithubError#message a Hash and not a String? #44
Comments
Hi Darren, yeah this looks too brittle for what it needs to be. I will try to tackle this one and simplify it asap, however in the meantime if you have any suggestions how this should be handled please send in a patch. Thanks Piotr |
Thank you for your feedback. I may send you a patch when I have time :) |
Hi Darren, I've spent some time on this issue and rewrote the error handling to always return a string message. All the response decoding and message extraction now happens inside the gem, therefore your code can be simplified as follows: begin
#Do something with github_api gem
rescue Github::Error::GithubError => e
puts e.message => #e.g. 'key is already in use'
end Please note that you can further refine your error catching to only GitHub service errors by using I hope this helps. Please download the v0.6.4 gem release and let us know if that's working for you? P |
I tried it out and it works! The message content includes the HTTP verb and the url (as well as the error message) by the way, for example, I received: "POST https://api.github.com/repos/jensendarren/try_git/keys?access_token=003betc7uwehblahblah7wj: 422 key is already in use" Anyway, e.message.class is now a String which is great! Thanks Peter! |
Awesome, I figured that having the explicit request information together with error message will make it more convenient for future debugging. Glad it works for you. |
I want to test a method which is using this gem. However, since the message attribute is a Hash it's difficult to test and makes the code a bit brittle too. Here's what I have. Is there a better way to do this?
The text was updated successfully, but these errors were encountered: