5

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)
  • There is only two excepts now, but it should be more for the several request exceptions. But already the view method is bloated up by this. Is there a way to forward the except handling to a separate error method?
  • There is also the problem, that I need here to call the render message for each except, I would like to avoid that.
  • And here I also repeat for each except "could not connect to your box because", that should be set once when there appeared any exception.

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...

Asara
  • 2,791
  • 3
  • 26
  • 55
  • ok this might be a duplicate of https://stackoverflow.com/questions/38084360/separating-except-portion-of-a-try-except-into-a-function, there is proposed to handle it in a function with `if type(e).__name__ == 'ReadError'`, is this really the pythonic/django way? – Asara Apr 17 '20 at 12:41
  • It's not pythonic. Generally using `__dunder__` methods is a last resort / workaround thing, and checking the class name in particular is very hacky. Try [using `isinstance` instead](https://stackoverflow.com/questions/1549801/what-are-the-differences-between-type-and-isinstance) – Kevin Languasco Apr 29 '20 at 15:29

2 Answers2

10

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.

Kevin Languasco
  • 2,318
  • 1
  • 14
  • 20
  • 1
    This is actually a very good solution. Respects the DRY principle, clearly divides class responsibility throighout. Think it should be accepted. – Julio Cezar Silva Apr 28 '20 at 13:46
  • Iam not very convinced by this, actually I was looking for a more general solution, so I will also have a look into `process_exception()`, so but I hope I can adjust your solution to my needings. And because it seems that there is no better solutions yet, I will accept it – Asara Apr 29 '20 at 09:07
  • @Asara in what way this doesn't cover your use case? – Kevin Languasco Apr 29 '20 at 15:46
0

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)
RHSmith159
  • 1,823
  • 9
  • 16
  • 2
    still, this would make it necessary to put a lot of error handling code into the view method. I my perception, I should immediately leave this method if an exception error happens becausethen there went something wrong and this method is no longer responsible for dealing with it – Asara Apr 17 '20 at 13:01