Feature #18145: Rescue by nested exception

• Status: Closed
• Assignee: -
• Target version: -

[svoop (Sven Schwyn)]

The introduction of Exception#cause helps a lot when debugging nested errors.

Same goes for wrapped errors. I'm not really sure whether such wrapped errors are an advisable pattern to begin with, feel free to comment on this, but I've used it in a couple of vendored gems dealing with payment providers.

Here's some simplified code to illustrate. The Payment class deals with all the gory things such as authorization, coercion or API quirks. A simplified version might look like this:

require 'rest_client'

module Provider
  class FindError < StandardError; end

  class Payment
    attr_reader :id, :amount

    private_class_method :new

    def initialize(id, amount)
      @id, @amount = id, amount
    end

    def self.find(id)
      response = RestClient.get('https://api.provider.com/payments', params: { id: id })
      body = JSON.parse(response.body)
      new(id, body.fetch('amount'))
    rescue
      raise FindError
    end
  end
end

You can easily rescue from anything going wrong when loading a payment:

begin
  Provider::Payment.find(123)
rescue FindError
  ...
end

However, you might want to rescue differently for some specific causes (e.g. not found) but not for others (e.g. timeout):

begin
  Provider::Payment.find(123)
rescue FindError => error
  if error.cause.instance_of? RestClient::NotFound
    ...
  else
    ...
  end
end

How about allowing to rescue by nested exception with a syntax like?

begin
  Provider::Payment.find(123)
rescue FindError & RestClient::NotFound
  ...
rescue FindError
  ...
end

[jeremyevans0 (Jeremy Evans)]

svoop (Sven Schwyn) wrote:

How about allowing to rescue by nested exception with a syntax like?

begin
  Provider::Payment.find(123)
rescue FindError & RestClient::NotFound
  ...
rescue FindError
  ...
end

That particular syntax is already valid. It just depends on FindError.&(RestClient::NotFound) returning a valid exception class.

You can already implement this support yourself:

def Exception.&(cause)
  klass = self
  Class.new do
    define_singleton_method(:===) do |exc|
      klass === exc && cause === exc.cause
    end
  end
end

[svoop (Sven Schwyn)]

@jeremyevans0 (Jeremy Evans) Nice one, thanks for the hint! I'll add this to a ProviderError and inherit from there. I guess this use case is too specific to add this self.& to StandardError or even Exception upstream?

[jeremyevans0 (Jeremy Evans)]

svoop (Sven Schwyn) wrote in #note-2:

@jeremyevans0 (Jeremy Evans) Nice one, thanks for the hint! I'll add this to a ProviderError and inherit from there. I guess this use case is too specific to add this self.& to StandardError or even Exception upstream?

That's not for me to decide, but I would be against it. The need seems very limited to me, so the benefit seems low. Also, the approach requires allocating a new class for each exception where it needs to be tested, so it's suboptimal from a performance standpoint. A better performing approach would cache the equivalent of the FindError & RestClient::NotFound expression into a constant, and use that in the rescue clause.

[byroot (Jean Boussier)]

I'll add that this isn't really a good design because it leaks the underling implementation. The caller of Provider::Payment shouldn't have to know that RestClient was used.

The more elegant way to do this would be:

module Provider
  class FindError < StandardError; end
  class NotFoundError < FindError; end

  class Payment
    attr_reader :id, :amount

    private_class_method :new

    def initialize(id, amount)
      @id, @amount = id, amount
    end

    def self.find(id)
      response = RestClient.get('https://api.provider.com/payments', params: { id: id })
      body = JSON.parse(response.body)
      new(id, body.fetch('amount'))
    rescue RestClient::NotFound
      raise NotFoundError
    rescue
      raise FindError
    end
  end
end

begin
  Provider::Payment.find(123)
rescue NotFoundError
  ...
rescue FindError
  ...
end

[svoop (Sven Schwyn)]

Both valid points, thanks for your suggestions... this issue can be closed IMO.

[duerst (Martin Dürst)]

• Status changed from Open to Closed

svoop (Sven Schwyn) wrote in #note-5:

Both valid points, thanks for your suggestions... this issue can be closed IMO.

Closed at request of OP.