Fail tests when unraisable exceptions occur

[Adam Johnson]

Following django-developers discussion: ​https://groups.google.com/g/django-developers/c/ea21KjiGiY4/m/7HTnQaS8BwAJ

Django's test runner should install an unraisable exception hook and fail the test run if it is ever triggered. The check could be done just once at the end of the test run (in subprocesses when run in parallel), because unraisable exceptions already print their traceback by default.

[Adam Johnson]

There's a reference implementation in pytest we might be able to copy: ​https://github.com/pytest-dev/pytest/pull/8055

Additionally here's an implementation I've used - calling the context manager in an overridden DiscoverRunner.run_tests to wrap the whole test run:

@contextmanager
def fail_if_any_unraisable_exceptions():
    orig_unraisable_hook = sys.unraisablehook
    hook_called = multiprocessing.Value("b", lock=False)
    hook_called.value = 0

    def hook(unraisable, /):
        hook_called.value = 1
        orig_unraisable_hook(unraisable)

    sys.unraisablehook = hook

    try:
        yield
    finally:
        sys.unraisablehook = orig_unraisable_hook

        if hook_called.value:
            print(
                "❌ Unraisable exceptions reported, failing test run - see above.",
                file=sys.stderr,
            )
            sys.exit(1)

[Mariusz Felisiak]

Tentatively accepted. I would like to check this in practice.

[Chris Jerdonek]

One comment: the sys.unraisablehook docs say:

Called when an exception has occurred but there is no way for Python to handle it. For example, when a destructor raises an exception or during garbage collection (gc.collect()).

Thus, with this patch, if we disable garbage collection as in this PR:
​https://github.com/django/django/pull/14142
then it seems like we won't get the benefit of this patch in checking for exceptions that occur during garbage collection.

[Adam Johnson]

Exceptions in __del__ are only one kind of unraisable exception. The other kind I know of that motivated me to look into them are from async tasks that were not joined by the task that spawned them.

Python already prints tracebacks for unraisable exceptions, the main point of this ticket is to ensure that such tracebacks in tests do not get missed because the tests exit with a 0 status code. The default behaviour looks like this:

$ cat test.py
class Naughty:
    def __del__(self):
        1 / 0


Naughty()

$ python test.py
Exception ignored in: <function Naughty.__del__ at 0x1015da8b0>
Traceback (most recent call last):
  File "/Users/chainz/tmp/test/test.py", line 3, in __del__
    1 / 0
ZeroDivisionError: division by zero

$ echo $?
0

The PR disabling gc affects only objects in cycles, which are a minority. Moreover all garbage is still collected at interpreter shutdown, so any unraisable exceptions would have their traceback printed at that point, although they indeed not be picked up by a sys.unraisablehook installed only for the duration of the test run. That said, the gc PR affects only the Django test suite which has no unraisable exceptions at the moment, and it's likely if any were added someone would spot them.

[Thomas Grainger]

You should be able to run gc.collect in a loop until no more objects get collected just before you unset your unraisablehook

Or just a constant number of gc.collect cycles 4 should be enough for anyone
​https://github.com/python-trio/trio/blob/7b86d20549bba2629eb929a72022a19b999c91d5/trio/_core/tests/tutil.py#L55