ApiError as return object or exceptions? An open discussion

[hholst80]

I do not know if it is part of the coming redesign but at some point you @bcb mentioned that you are looking into making ApiError a return object instead of using it as an exception. Just to be clear, I am not trying to say that either design is better than the other, I just think it is an interesting technical discussion. To that end I hope I can provide "anecdotal evidence" for exceptions as error mechanism.

I did some refactoring today on an API and I kept the backend modules as is, without exposing the methods to the rpc server. (the API is some boilerplate handling logic on top of K8s). After the refactoring the new public RPC method became something of the form

def api_method():
    be1.method()
    be2.method()
    # ...
    # Empty response = all ok

Since the methods all throw an ApiError on error, it became trivial to refactor the code using the existing pieces from the old backend. Otherwise I would have to inject error handling logic in each step. A small win perhaps, but I liked it. Looks cleaner than the typical golang/C boilerplate.

I have not thought about this in any depth so it would be great to hear from others what you and other users see as the pros/cons of the two different approaches to error handling? Consistency with other frameworks/libraries in this space? More idiomatic Python style?

Cheers!

[st31ny]

I actually agree with you, exceptions make it quite easy to "error-out" deep in the code. On the other hand, languages that enforce strict type handling and have support to easily work with error results, such as Rust, the API of individual functions might look cleaner and more consistent and the caller cannot mess it up. The C/golang approach pleases me the least as it requires boilerplate everywhere and is easy to get wrong.
So, since we are writing in Python here and Python has exceptions and is dynamic (i.e., cannot check correct type handling at compile time), I'd clearly advocate for the exception style. This is indeed also what other libs do, e.g., aiohttp. The example of aiohttp also shows, that both concepts are not exclusive, so returning or throwing an ApiError could mean the same thing.

[bcb]

I looked at a few web frameworks this morning, they all allow you to either return a Response or raise an Exception. There's no reason why we can't support both.

I would suggest returning a response though where you can. The job of the method is to return a response, so we should do that where we're able to, in my opinion.

[hholst80]

Maybe an InvalidParams exception would also be useful.

The API I am putting out there is not mature enough to have those high quality concerns yet. ;-) I will let you know once and if I reach that point.

[bcb]

Seems aiohttp even has exceptions for success 😵

Let’s stick with one exception for now, adding more later won’t be a breaking change.

[st31ny]

Well, right now it's a breaking change since InvalidParamsError, ApiError and the like have been removed.

[bcb]

We could add them for backwards compatibility, but at the end of the day there will be some breaking changes in this release, I think that's the whole point of making a new major version. And one of the main changes in this release will be in how you write the methods. Nothing's been decided yet though and this discussion is really helpful. I'll make a post on how the proposed methods will look so we can pick it apart.

[st31ny]

Sure sounds good! I am with you: It should be possible to drop old features at some point.

[bcb]

This is what I'm proposing for methods in version 5, at this stage:

from jsonrpcserver.result import Error, InvalidParams, Result, Success

@method
def fruits(color: str) -> Result:
    if color != "yellow":
        return InvalidParams("No fruits of that colour")
    if some_other_issue:
        return Error(-1, "Some other issue")
    return Success("banana")

The return value "Result" is a union of Success and Error (Union[Success, Error]). InvalidParams is an Error type. You can also raise a JsonRpcError exception, or whatever we want to call that, as we decided above. Any other exception will result in an "Internal error" jsonrpc response.

The library will convert the Result type into a Response (which adds the original request id).

[st31ny]

Sounds good to me!

[bcb]

I've added JsonRpcError exception for now (see #178) but I'm still unsure about it. We shouldn't raise exceptions. The function allows you to return an Error so we should do that.

[st31ny]

Why shouldn't we raise exceptions?

[hholst80]

I think the point of my initial post was that exception enables a clean method to return from a sequence of calls without the if err := methodCall(); if err: return Error pattern often to see in C/golang. In either say, I think that raise or return should probably be semantically equivalent, but it might require the ErrorObject etc to inherit also from the Exception class.

[bcb]

Because having a function that returns a value for some inputs but not for others, adds complexity, makes it harder to understand.

You should strive to avoid partial functions and instead write total ones. This makes it much easier to reason about your code and makes "if your code compiles, it probably works" true for your code more often. https://wiki.haskell.org/Partial_functions

[bcb]

There are better ways of short-circuiting your code than c/go/exceptions in other languages, but Python doesn’t have it so you can use the exception. It’s merged in 👍

[bcb]

You can return an Error or raise a JsonRpcError, either will work.