Opened 7 years ago

Closed 7 years ago

#28244 closed Uncategorized (wontfix)

Exceptions derived from BaseException instead of Exception are not caught by convert_exception_to_response()

Reported by: alexpirine Owned by: nobody
Component: Uncategorized Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by alexpirine)

Hello,

Python documentation states:

In Python, all exceptions must be instances of a class that derives from BaseException.

Deriving exceptions from Exception is a recommendation, but does not seem to be an absolute requirement:

programmers are encouraged to derive new exceptions from the Exception class or one of its subclasses, and not from BaseException.

So, if BaseException is used instead of Exception, Django's convert_exception_to_response doesn't catch it, and crashes instead:

def convert_exception_to_response(get_response):
    @wraps(get_response)
    def inner(request):
        try:
            response = get_response(request)
        except Exception as exc:
            response = response_for_exception(request, exc)
        return response
    return inner

If you run it on gunicorn with nginx, which is often the case, you will see upstream prematurely closed connection while reading response header from upstream errors, without any additional info, and without any logs whatsoever, even if you properly configured a logging system.

Only running manage.py runserver will display the full stack trace:

Traceback (most recent call last):
  File "…/python3.6/wsgiref/handlers.py", line 137, in run
    self.result = application(self.environ, self.start_response)
  File "…/django/contrib/staticfiles/handlers.py", line 63, in __call__
    return self.application(environ, start_response)
  File "…/django/core/handlers/wsgi.py", line 157, in __call__
    response = self.get_response(request)
  File "…/django/core/handlers/base.py", line 124, in get_response
    response = self._middleware_chain(request)
  File "…/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "…/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "…/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  [… and so on, depending on how much middleware you have …]
  File "…/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "…/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "…/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "…/django/views/decorators/http.py", line 40, in inner
    return func(request, *args, **kwargs)
  File "…/your_project/views.py", line 42, in your_func
    …
your_project.YourErrorInheritedFromBaseException: Your error message

Of course I fixed my code by inheriting from Exception.

But I wonder if catching BaseException instead of Exception would be a good change?

Change History (3)

comment:1 by alexpirine, 7 years ago

Summary: Exceptions inheriting from BaseException instead of Exception are not correctly handledExceptions derived from BaseException instead of Exception are not correctly handled

comment:2 by alexpirine, 7 years ago

Description: modified (diff)

comment:3 by Tim Graham, 7 years ago

Resolution: wontfix
Status: newclosed
Summary: Exceptions derived from BaseException instead of Exception are not correctly handledExceptions derived from BaseException instead of Exception are not caught by convert_exception_to_response()

Perhaps the Python docs need amending if you got the impression that user exceptions may inherit from BaseException. As far as I know, that's not the case. From later on in the document you linked, "[BaseException] is not meant to be directly inherited by user-defined classes (for that, use Exception)." And from the docs for Exception: "All user-defined exceptions should also be derived from this class."

Note: See TracTickets for help on using tickets.
Back to Top