7

I'm developing a Ruby on Rails app. The app contains a service wrapping an external REST API, called from a controller, with several possible error states. The current implementation returns the response body on success and raises a service specific catch-all exception otherwise.

I want to refactor this to be able to distinguish between network errors et al, and authorization errors which are caused by the client supplying invalid parameters. What is the most idiomatic way to do this? What has resulted in the most maintainable code in your experience?

Below are some alternatives I've considered.

Exceptions throughout

It is recommended that a library should have one subclass of StandardError or RuntimeError and have specific exception types inherit from it. This allows the user to rescue a generic exception type to catch all exceptions the library may raise even if future versions of the library add new exception subclasses.

From the official Ruby documentation.

The complete code for my service is currently 39 lines and can't be considered a library, but this strategy might still be applicable. A possible implementation is listed below.

class MyService
  def self.call(input)
    res = do_http_call input

    if res and res.code == 200
      res.body
    elsif res and res.code == 401
      fail MyServiceAuthenticationError
    else
      fail MyServiceError
    end
  end
end

class MyServiceError < StandardError
end

class MyServiceAuthenticationError < MyServiceError
end

Coming from other languages this approach doesn't feel right. I've often heard the mantra "reserve exceptions for exceptional cases", for instance in Code Complete (Steve McConnell, 2nd edition, p. 199):

Throw an exception on for conditions that are truly exceptional

Exceptions should be reserved for conditions that are truly exceptional – in other words, for conditions that cannot be addressed by other coding practices. Exceptions are used in similar circumstances to assertions – for events that are not just infrequent but for events that should never occur.

Are exceptions really for exceptional errors? is a discussion of this topic. The answers provide varying advice, and S.Lott's answer explicitly states "Don't use exceptions to validate user input," which I think is roughly what the strategy outlined above amounts to.

Symbols for "non exceptional" errors

My first intuition is to use exceptions for errors I want to bubble up the stack and symbols for results the caller can expect and wants to handle.

class MyService
  def self.call(input)
    res = do_http_call input

    if res.code == 200
      res.body
    elsif res.code == 401
      :invalid_authentication
    else
      fail MyServiceError
    end
  end
end

class MyServiceError < StandardError
end

Just like Exceptions throughout, this is easy to extend with additional errors.

It could however lead to maintainability problems. If a new symbol return value is added and a caller isn't modified the error symbol could silently be interpreted as successful return since the success return value is a string. I don't know how realistic this is in practice though.

Additionally, this approach can be considered stronger coupled to its caller. Whether an error should bubble up the call stack or be handled by the immediate caller is arguably not something the callee should concern itself with.

False on error

An example of this approach is ActiveRecord::Base#save.

  • If the operation is successful it returns the result, or true in the case of #save.
  • If validations fail it returns false.
  • If some type of unexpected errors occur, like encoding fields with UTF-8 in #save, an exception is thrown.
class MyService
  def self.call(input)
    res = do_http_call input

    if res.code == 200
      res.body
    elsif res.code == 401
      false
    else
      fail MyServiceError
    end
  end
end

class MyServiceError < StandardError
end

I generally dislike this strategy as false doesn't carry any semantic meaning and it's impossible to distinguish between errors.

Another way

Is there another superior way?

1

Maybe you're looking for something like a status object.

For example:

class MyService
  def self.call(input)
    Client.call(input) do |status|
      status.on_success do |response|
        response.body
      end

      status.on_not_authorized do
        # do something when not authorized
      end

      status.on_error do
        # do something then there's an error
      end
    end
  end
end

class Client
  def self.call(input)
    res = do_http_call(input)

    if res.code == 200
      yield RequestStatus.success(res)
    elsif res.code == 401
      yield RequestStatus.not_authorized
    else
      yield RequestStatus.error
    end
  end
end

class RequestStatus
  def self.success(response)
    new(:success, response)
  end

  def self.not_authorized
    new(:not_authorized)
  end

  def self.error
    new(:error)
  end

  def initialize(status, response = nil)
    @status = status
    @response = response
  end

  def on_success
    yield(@response) if @status == :success
  end

  def on_not_authorized
    yield if @status == :not_authorized
  end

  def on_error
    yield if @status == :error
  end
end

I don't know if this pattern is better per se, but if you want to avoid bubbling up exceptions, or passing symbols around, this might be a good alternative.

  • Do you have any examples of projects using this approach or people advocating it? – jacwah Jan 8 '17 at 15:07
1

A realy good way I've seen for organizing exceptions in a Ruby library is in sferik's twitter library.

https://github.com/sferik/twitter/blob/master/lib/twitter/error.rb

By using Ruby's dynamic class creation mechanism with Class.new(ParentClass) It makes it very easy to reason about the class hierarchy.

Client related exceptions inherit from ClientError. Server related exceptions inherit from ServerError. ClientError and ServerError both inherit from Twitter::Error

HTTP response codes are mapped to error classes and raised upon receiving the HTTP response:

@parser = Http::Parser.new(http_response)
error = Twitter::Error::ERRORS[@parser.status_code]
raise error if error

I've abreviated the code to only show the important parts:

module Twitter
  class Error < StandardError
    ...    
    ClientError = Class.new(self)
    BadRequest = Class.new(ClientError)
    Unauthorized = Class.new(ClientError)
    RequestEntityTooLarge = Class.new(ClientError)
    NotFound = Class.new(ClientError)
    NotAcceptable = Class.new(ClientError)
    UnprocessableEntity = Class.new(ClientError)
    TooManyRequests = Class.new(ClientError)

    Forbidden = Class.new(ClientError)
    AlreadyFavorited = Class.new(Forbidden)
    AlreadyRetweeted = Class.new(Forbidden)
    DuplicateStatus = Class.new(Forbidden)


    ServerError = Class.new(self)
    InternalServerError = Class.new(ServerError)
    BadGateway = Class.new(ServerError)
    ServiceUnavailable = Class.new(ServerError)
    GatewayTimeout = Class.new(ServerError)

    ERRORS = {
      400 => Twitter::Error::BadRequest,
      401 => Twitter::Error::Unauthorized,
      403 => Twitter::Error::Forbidden,
      404 => Twitter::Error::NotFound,
      406 => Twitter::Error::NotAcceptable,
      413 => Twitter::Error::RequestEntityTooLarge,
      422 => Twitter::Error::UnprocessableEntity,
      429 => Twitter::Error::TooManyRequests,
      500 => Twitter::Error::InternalServerError,
      502 => Twitter::Error::BadGateway,
      503 => Twitter::Error::ServiceUnavailable,
      504 => Twitter::Error::GatewayTimeout,
    }.freeze
    ...
  end
end

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.