chargebee-ruby
chargebee-ruby copied to clipboard
only rescue RestClient exceptions
Currently all errors coming from the depths of rest-client are rescued and translated to IOError. I suggest to change it:
- It's a bad practice to rescue
Exception. If you want to rescue all exceptions, rescueStandardError. Here's why. - As in the scenario described in #15, an error may occasionally happen somewhere deep in rest-client, that has nothing to do with network communication, but results solely from gems incompatibility or other client-side software misfortune. It is really misleading to mask such errors as
IOError.
I believe the right thing to do is to only handle exceptions raised by rest-client (which fortunately have a common ancestor RestClient::Exception). rest-client already does an awesome job handling various exceptions from the layers below and translating them to instances of it's own exception types, so you can be assured that exceptions of other types aren't "mere IO errors" and signal deeper problems.
One line code change to improve error handling. Looks good to me. Ship it!
@cb-navaneedhan - can this get merged?
@cb-nithins can we merge this one?
Ping
@cb-alish would you review this one as well?
Hi @sobrinho, this is no longer relevant since we've moved away from rest-client. Are you seeing a similar issue with the new HTTP client? Let me review and add support if it's still applicable. Thanks.
@cb-alish lib/chargebee/rest.rb still contains several occurrences of the rescue Exception antipattern, which is discussed in my first post above.
Personally I'm not involved in any ChargeBee integration anymore, so I don't really care, but if you care about your users/integrators/customers, it's definitely something worth fixing.
... but it's true that this PR as such is now outdated and makes no more sense. Closing.