httpi
httpi copied to clipboard
savonrb
Unified HTTP Exceptions?
I would love to have httpi catch (rescue) and re-throw all adapter exceptions so that client code (e.g. my Savon SOAP client) could be a bit cleaner in its exception handling.
A connection timeout, a connection refused, a internal server error... they are all different between the libraries and also not always in a "uniform" structure within each adapter.
In a SOAP client I may want to distinguish all http related errors from SOAP errors and respond to them in a certain way as a group. I may want to try a backup endpoint, for example. Something I would not want to do for SOAP exceptions but which might be a good idea for timeouts and server errors.
I haven't looked at all adapters in detail to inventory the possible exceptions in full. Excon had them all nicely bundled together... about 45-48 of them it looks like. https://github.com/geemus/excon/blob/master/lib/excon/errors.rb
What do you think? Good idea? Bad idea? A lot of work?
For stuff like Timeout etc., this should definitely be done. But for HTTP errors, we already got Response#error? and Response#code.
The thing is that at least some of the http libs throw exceptions on "bad" response codes like "not acceptable" or "forbidden".
I am not sure they can all be supressed. Using httpi with Savon and setting " Savon.raise_errors = false" I still get exceptions for network activity.
The point if to be able to catch all relevant exceptions in a clean manner. Catching all exceptions that are subclasses of, say, HTTPI::NetworkError or some such would be very nice.
I am playing with implementing an Excon adapter... Maybe that will imprive my insight into httpi a bit.
@eimermusic, I believe Net::HTTP will raise certain exceptions under Exception rather than StandardError when they are network related. This means that they will not be rescued by the typical rescue => e. Rather, you need to rescue Exception => e.
I agree with you this would be a good feature, and I would think the nested_exceptions gem could be used to capture the underlying exception.
I could see a way this could be done more easily.
In Savon, wrap use of httpi with
begin
httpi.call
rescue Exception => e
raise SavonError(e)
end
As @lgustafson points out, exception chaining is needed, but this would allow all underlying connection and transmission errors to be caught and rethrown as one unified error. Callers could always inspect the chained error to discover the underlying cause.
I've done something similar in a work project by wrapping my class' method_missing delegate call to a Savon model underneath. Everything that can go wrong there is a kind of ServiceError.
BTW, chaining is easy if you own the exception class, no need to use the gem if you don't want the fancy backtrace (which may not be as useful to external Savon users vs. Savon maintainers):
class SavonError < RuntimeError
def initialize(msg=nil, cause=nil)
@cause = cause
super(msg)
end
def to_s
super.to_s + ": caused by " + @cause.to_s
end
end
or something similar.
Just some ideas.
Your great idea give me a nice idea! Thanks @coldnebo ... if i found some time after lunch, i'll start something crazy.
Looks like this ticket can actually be closed but I was wondering why HTTPI::ConnectionError doesn't extend from HTTPI::Error like all the others do? Seems like an oversight?
https://github.com/savonrb/httpi/blob/master/lib/httpi.rb#L91
Ah, just realized it's a module and not a class, in order to support this usage:
https://github.com/savonrb/httpi/blob/master/lib/httpi/adapter/net_http.rb#L47-L49
Though I don't really understand the reason for this either. Why not just raise ConnectionError in the same way as the others?
Hi @chetan. This is really old and i don't know why this ConnectionError is a module. Maybe is something related to savon. :confused:
@chetan -- glad you brought this up.
Seems like the behavior to handle ConnectionError in this way came into the codebase in December 2012. I don't think there's a strong argument to be made for doing it this way -- raising ConnectionError like you suggested seems a lot better to me. Consistency is better than clever code, in my humble opinion.
The reason for extending last error $! with a module (e.g. ConnectionError) is that exception can be catched with a module now:
module ErrTrait; end
class MyErr < StandardError; end
begin
b = MyErr.new("msg")
b.extend ErrTrait
fail b
rescue ErrTrait => e
puts e.inspect # #<MyErr: msg>
end
e.is_a?(ErrTrait) returns true now, and ErrTrait acts like a common 'tag' for different kinds of exceptions.
Another solution is: reraise errors from rescue block using different error class. But in this case it is harder to detect original error which caused this error to happen (Exception#cause is available since Ruby 2.1 to do that). It is also a breaking change, because library users may already rely on catching some specific error classes.
