Error handling

I've been thinking about error handling lately. I'm working with a Flask route that looks something like this:

@app.route("/api/parse_thing", methods=["POST"])
def parse_thing():
    ...
    url = "{settings.parse_server}/some/parsing/aid"
    params = {"foo": "the lion", "bar": "the witch"}
    response = requests.get(url, data=json.dumps(params), timeout=30)
    if response.error:
        logger.error("error from parse server was %s", response.content)
        return {
            "message": "Bad thing",
        }, 500
    data = response.json()["tree"]
    ...

This route makes several a few API calls like this. It's a very long method, about a hundred lines, and I need to add code to it. Each API call takes up about twenty lines each. I'm thinking about the right way to cut that down, and these API calls are one of the places I'd like to refactor out.

One possibility is to factor out everything except handling the response. That is, change the above to something like this:

@app.route("/api/parse_thing", methods=["POST"])
def parse_thing():
    ...
    response = call_api(params)
    if response.status_code != HTTPStatus.OK:
        logger.error("error from parse server was %s", response.content)
        return {
            "message": "Bad thing",
        }, 500
    data = response.json()["tree"]
    ...

Then the longest part of the API calling code gets moved. That part is great.

However, this solution feels leaky. call_api() is just returning the requests object. That means the caller has to care about requests even though it isn't directly calling into that library. First, the caller is forced to check the response's status code, rather than checking for an error. Second, on a successful API call, the caller has to finish the translation from the response content into application objects – in this case, calling .json() and fetching the tree key. The clients aren't entirely shielded from the details of the API call.

How can we do better? Well, I'd like to push the "on success" logic into call_api(). However, there's this error handling bit in the middle that's getting in the way. We push it down into call_api() by having the function return data, error and handle the error like so:

@app.route("/api/parse_thing", methods=["POST"])
def parse_thing():
    ...
    data, error = call_api(params)
    if error:
        logger.error("error from parse server was %s", error)
        return {
            "message": "Bad thing",
        }, 500
    ...

That seems fine as is. It's a little funny in that I want the log message to remain the same, so error must be a string here. Allowing a string error is probably fine.

However, with type-checking this gets ugly. We have to either return placeholder data on an error and leave it to the caller to check the error code, or return a messy type and force the caller to deal with the mess. The placeholder data solution seems a little better, but neither seems great.

What else could we do? We could throw an exception if the API call fails. I'd like to avoid throwing a common exception like ValueError because I'd like callers to be able to catch and respond to API call failures separate from errors in the backend code. So, say we define a custom error ParseAPIError like so:

class ParseAPIError(Exception):
    pass

We can then raise this in call_api() and catch it when we call:

@app.route("/api/parse_thing", methods=["POST"])
def parse_thing():
    ...
    try:
        data = call_api(params)
    except ParseAPIError as e:
        logger.error("error from parse server was %s", e.message)
        return {
            "message": "Bad thing",
        }, 500
    ...

This has the virtue that it won't fail silently. We can't get away with refusing to handle the error. On the other hand it is slightly longer than the error code solution. Overall though this seems like the best solution I've come up with so far.

This does however keep a nasty feature of the other solutions. The problem is that assuming we do this for each API call separately, the error handling remains mixed in with the "happy path" logic. That's not great for readability.

Is there a better way to do this? It's something to think about.