[Django] #35960: Improve most tracebacks with exception chaining

[Django]

#35960: Improve most tracebacks with exception chaining
-------------------------------------+-------------------------------------
Reporter: Bartosz | Owner: Bartosz Sławecki
Sławecki |
Type: | Status: assigned
Cleanup/optimization |
Component: | Version: dev
Uncategorized | Keywords: exception handling,
Severity: Normal | exception chaining, exceptions
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 1 |
-------------------------------------+-------------------------------------
Hi! This is my first time using Django.

During configuring my project, I happened to run into a combined traceback
caused by the inavailability of `psycopg`—it's a typical traceback a
developer using Django may get, so I'm posting it here with most lines
truncated to emphasize the conjuctions between tracebacks:

{{{
Traceback (most recent call last):
(...)
File "/home/bswck/Projects/obiadomat/.venv/lib/python3.12/site-
packages/psycopg/pq/__init__.py", line 109, in import_from_libpq
raise ImportError(
ImportError: no pq wrapper available.
(...)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/home/bswck/Projects/obiadomat/.venv/lib/python3.12/site-
packages/django/db/backends/postgresql/base.py", line 27, in <module>
import psycopg2 as Database
ModuleNotFoundError: No module named 'psycopg2'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
(...)
File "/home/bswck/Projects/obiadomat/.venv/lib/python3.12/site-
packages/django/db/backends/postgresql/base.py", line 29, in <module>
raise ImproperlyConfigured("Error loading psycopg2 or psycopg module")
django.core.exceptions.ImproperlyConfigured: Error loading psycopg2 or
psycopg module
}}}

As we read in the traceback, the `ImproperlyConfigured` exception occured
during handling the exception `ModuleNotFoundError: No module named
'psycopg2'`. While this is technically true, I think we'd be happy to
improve this traceback to this:

{{{
Traceback (most recent call last):
(...)
File "/home/bswck/Projects/obiadomat/.venv/lib/python3.12/site-
packages/psycopg/pq/__init__.py", line 109, in import_from_libpq
raise ImportError(
ImportError: no pq wrapper available.
(...)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/home/bswck/Projects/obiadomat/.venv/lib/python3.12/site-
packages/django/db/backends/postgresql/base.py", line 27, in <module>
import psycopg2 as Database
ModuleNotFoundError: No module named 'psycopg2'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
(...)
File "/home/bswck/Projects/obiadomat/.venv/lib/python3.12/site-
packages/django/db/backends/postgresql/base.py", line 29, in <module>
raise ImproperlyConfigured("Error loading psycopg2 or psycopg module")
from e
django.core.exceptions.ImproperlyConfigured: Error loading psycopg2 or
psycopg module
}}}

...where we instead read that the `ModuleNotFoundError: No module named
'psycopg2'` exception was the reason we got the `ImproperlyConfigured`
exception. I used exception chaining
https://docs.python.org/3/tutorial/errors.html#exception-chaining for this
demonstration.

I flicked through the codebase @ the main branch and found a surprisingly
large amount of breakpoints that could be improved with chained
exceptions.

I offer to create a series of PRs that will close this ticket. I want to
distribute the work needed to add this improvement and make one PR per
every Django component to ensure that each of these PRs is carefully
reviewed and confirmed by the right people who specialize in certain areas
and can easily decide where a reason could be hidden or not.

Notes to self:
No `from` clause in a `raise` statement means that the user gets a
traceback with the ''during handling ..., another exception occured''
conjuction between tracebacks. Hence, let's add the `from None` clause
only to very obvious cases, like `KeyError`s from dictionary access, or
some super-simple `AttributeError`s (we'd like to facilitate debugging in
cases where, for instance, a specialized attribute error occured inside a
property–`from None` would hide the original error and not be helpful). We
wouldn't like to add `from None` to any
`ImportError`/`ModuleNotFoundError` (`ImportError` subclass) handling
blocks, as they may lose information in the message of a transitive
exception that should be blamed.

`from <captured exception variable name>` in the vast majority of clauses
ensures that the tracebacks we get in the future versions of Django
affected by the PR remain around the same as those from the past versions,
so they are 'backward-compatible' in a sense (please don't read this
sentence literally, YKWIM).

Please LMK what you think about this suggested improvement!
--
Ticket URL: <https://code.djangoproject.com/ticket/35960>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35960: Improve most tracebacks with exception chaining
-------------------------------------+-------------------------------------

Reporter: Bartosz Sławecki | Owner: Bartosz
Type: | Sławecki
Cleanup/optimization | Status: assigned
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: exception handling, | Triage Stage:
exception chaining, exceptions | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------

Description changed by Bartosz Sławecki:

Old description:

New description:

(including the one above).

I offer to create a series of PRs that will close this ticket. I want to
distribute the work needed to add this improvement and make one PR per
every Django component to ensure that each of these PRs is carefully
reviewed and confirmed by the right people who specialize in certain areas
and can easily decide where a reason could be hidden or not.

Notes to self:
No `from` clause in a `raise` statement means that the user gets a
traceback with the ''during handling ..., another exception occured''
conjuction between tracebacks. Hence, let's add the `from None` clause
only to very obvious cases, like `KeyError`s from dictionary access, or
some super-simple `AttributeError`s (we'd like to facilitate debugging in
cases where, for instance, a specialized attribute error occured inside a
property–`from None` would hide the original error and not be helpful). We
wouldn't like to add `from None` to any
`ImportError`/`ModuleNotFoundError` (`ImportError` subclass) handling
blocks, as they may lose information in the message of a transitive
exception that should be blamed.

`from <captured exception variable name>` in the vast majority of clauses
ensures that the tracebacks we get in the future versions of Django
affected by the PR remain around the same as those from the past versions,
so they are 'backward-compatible' in a sense (please don't read this
sentence literally, YKWIM).

Please LMK what you think about this suggested improvement!

--

--
Ticket URL: <https://code.djangoproject.com/ticket/35960#comment:1>

[Django]

and can easily decide where individual tracebacks causing the Django-end
tracebacks could be hidden or not.

Notes to self:
No `from` clause in a `raise` statement means that the user gets a
traceback with the ''during handling ..., another exception occured''
conjuction between tracebacks. Hence, let's add the `from None` clause
only to very obvious cases, like `KeyError`s from dictionary access, or
some super-simple `AttributeError`s (we'd like to facilitate debugging in
cases where, for instance, a specialized attribute error occured inside a
property–`from None` would hide the original error and not be helpful). We
wouldn't like to add `from None` to any
`ImportError`/`ModuleNotFoundError` (`ImportError` subclass) handling
blocks, as they may lose information in the message of a transitive
exception that should be blamed.

`from <captured exception variable name>` in the vast majority of clauses
ensures that the tracebacks we get in the future versions of Django
affected by the PR remain around the same as those from the past versions,
so they are 'backward-compatible' in a sense (please don't read this
sentence literally, YKWIM).

Please LMK what you think about this suggested improvement!

--
--

Ticket URL: <https://code.djangoproject.com/ticket/35960#comment:2>

[Django]

#35960: Improve most tracebacks with exception chaining
-------------------------------------+-------------------------------------
Reporter: Bartosz Sławecki | Owner: Bartosz
Type: | Sławecki

Cleanup/optimization | Status: closed
Component: Core (Other) | Version: dev
Severity: Normal | Resolution: duplicate

Keywords: exception handling, | Triage Stage:

exception chaining, exceptions, | Unreviewed
raise from |

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* component: Uncategorized => Core (Other)
* easy: 1 => 0
* keywords: exception handling, exception chaining, exceptions =>
exception handling, exception chaining, exceptions, raise from
* resolution: => duplicate
* status: assigned => closed

Comment:

Thanks for your interest in Django development.

This was discussed before, so I will mark as a duplicate of #31177.
Although it was decided there that a bulk update was not merited, new code
is always welcome to use `raise from` syntax, and in other cases a case-
by-case decision could be taken if there is a compelling reason (e.g.
silencing unwanted context via `from None`).
--
Ticket URL: <https://code.djangoproject.com/ticket/35960#comment:3>

[Django]

#35960: Improve most tracebacks with exception chaining
-------------------------------------+-------------------------------------
Reporter: Bartosz Sławecki | Owner: Bartosz
Type: | Sławecki
Cleanup/optimization | Status: closed
Component: Core (Other) | Version: dev
Severity: Normal | Resolution: duplicate
Keywords: exception handling, | Triage Stage:
exception chaining, exceptions, | Unreviewed
raise from |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Comment (by Bartosz Sławecki):

Replying to [comment:3 Jacob Walls]:

> Thanks for your interest in Django development.
>
> This was discussed before, so I will mark as a duplicate of #31177.
Although it was decided there that a bulk update was not merited, new code
is always welcome to use `raise from` syntax, and in other cases a case-
by-case decision could be taken if there is a compelling reason (e.g.
silencing unwanted context via `from None`).

OK, thanks for letting me know!
--
Ticket URL: <https://code.djangoproject.com/ticket/35960#comment:4>