Unified HTTP Exceptions? about httpi HOT 11 CLOSED

good idea and (possibly) a lot of work :)

from httpi.

For stuff like Timeout etc., this should definitely be done. But for HTTP errors, we already got Response#error? and Response#code.

from httpi.

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.

from httpi.

@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.

from httpi.

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.

from httpi.

Your great idea give me a nice idea! Thanks @coldnebo ... if i found some time after lunch, i'll start something crazy.

from httpi.

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

from httpi.

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?

from httpi.

Hi @chetan.
This is really old and i don't know why this ConnectionError is a module. Maybe is something related to savon. 😕

from httpi.

@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.

from httpi.

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.

from httpi.