CODEWITHSUNDEEP
pythondjangoexception
Asara
Asara

Reputation: 3404

how to improve exception handling in python/django

This is an example of my exception handling in a django project:

def boxinfo(request, url: str):
    box = get_box(url)
    try:
        box.connect()
    except requests.exceptions.ConnectionError as e:
        context = {'error_message': 'Could not connect to your box because the host is unknown.'}
        return render(request, 'box/error.html', context)
    except requests.exceptions.RequestException as e:
        context = {'error_message': 'Could not connect to your box because of an unknown error.'}
        return render(request, 'box/error.html', context)

I can solve it by something like this:

try:
    box.connect()
except Exception as e:
    return error_handling(request, e)

-

def error_handling(request, e):
    if type(e).__name__ == requests.exceptions.ConnectionError.__name__:
        context = {'error_message': 'Could not connect to your box because the host is unknown.'}
    elif type(e).__name__ == requests.exceptions.RequestException.__name__:
        context = {'error_message': 'Could not connect to your box because of an unknown error.'}
    else:
        context = {'error_message': 'There was an unkown error, sorry.'}
    return render(request, 'box/error.html', context)

and I could of course improve the error message thing then. But overall, is it a pythonic way to handle exceptions with if/else? For example I could not catch RequestException here if ConnectionError is thrown, so I would need to catch each requests error, that looks more like an ugly fiddling...

Upvotes: 5

Views: 3308

Answers (2)

Kevin Languasco
Kevin Languasco

Reputation: 2427

This is a use case for decorators. If it's something more general that applies to all views (say, error logging), you can use the Django exception middleware hook, but that doesn't seem to be the case here.

With respect to the repetitive error string problem, the Pythonic way to solve it is to have a constant base string with {replaceable_parts} inserted, so that later on you can .format() them.

With this, say we have the following file decorators.py:

import functools

from django.shortcuts import render
from requests.exceptions import ConnectionError, RequestException


BASE_ERROR_MESSAGE = 'Could not connect to your box because {error_reason}'


def handle_view_exception(func):
    """Decorator for handling exceptions."""
    @functools.wraps(func)
    def wrapper(request, *args, **kwargs):
        try:
            response = func(request, *args, **kwargs)
        except RequestException as e:
            error_reason = 'of an unknown error.'
            if isinstance(e, ConnectionError):
                error_reason = 'the host is unknown.'
            context = {
              'error_message': BASE_ERROR_MESSAGE.format(error_reason=error_reason),
            }
            response = render(request, 'box/error.html', context)
        return response

    return wrapper

We're using the fact that ConnectionError is a subclass of RequestException in the requests library. We could also do a dictionary with the exception classes as keys, but the issue here is that this won't handle exception class inheritance, which is the kind of omission that generates subtle bugs later on. The isinstance function is a more reliable way of doing this check.

If your exception tree keeps growing, you can keep adding if statements. In case that starts to get unwieldy, I recommend looking here, but I'd say it's a code smell to have that much branching in error handling.

Then in your views:

from .decorators import handle_view_exception

@handle_view_exception
def boxinfo(request, url: str):
    box = get_box(url)
    box.connect()
    ...

That way the error handling logic is completely separate from your views, and best of all, it's reusable.

Upvotes: 10

RHSmith159
RHSmith159

Reputation: 1592

could you have something like this:

views.py

EXCEPTION_MAP = {
    ConnectionError: "Could not connect to your box because the host is unknown.", 
    RequestException: "Could not connect to your box because of an unknown error.",
}

UNKNOWN_EXCEPTION_MESSAGE = "Failed due to an unknown error."


def boxinfo(request, url: str):
    box = get_box(url)
    try:
        box.connect()
    except (ConnectionError, RequestException) as e:
        message = EXCEPTION_MAP.get(type(e)) or UNKNOWN_EXCEPTION_MESSAGE
        context = {'error_message': message}
        return render(request, 'box/error.html', context)

You could then just expand the EXCEPTION_MAP and the except () for any other known exception types you're expecting to catch?

if you want to reduce the duplication of "Could not connect to your box because ...

You could maybe do:

views.py

BASE_ERROR_STRING = "Could not connect to your box because {specific}"

EXCEPTION_MAP = {
    ConnectionError: "the host is unknown.", 
    RequestException: "of an unknown error.",
}
UNKNOWN_EXCEPTION_MESSAGE = "Failed due to an unknown error."

def boxinfo(request, url: str):
    box = get_box(url)
    try:
        box.connect()
    except (ConnectionError, RequestException) as e:
        specific_message = EXCEPTION_MAP.get(type(e))
        if specific_message:
             message = BASE_ERROR_STRING.format(specific=specific_message)
        else:
             message = UNKNOWN_EXCEPTION_MESSAGE
        context = {'error_message': message}
        return render(request, 'box/error.html', context)

Upvotes: 0

Related Questions