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?