Opened 3 weeks ago
Closed 3 weeks ago
#37315 closed Bug (wontfix)
Backport #36487 to 5.2/6.0, or document partial callback crash with robust=True
| Reported by: | Bona Fide IT GmbH | Owned by: | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.2 |
| Severity: | Normal | Keywords: | |
| Cc: | Bona Fide IT GmbH | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
This is a follow-up to ticket #36487, which fixed an issue where transaction.on_commit error logging fails when using functools.partial with robust=True.
The fix landed in Django 6.1, but we ran into this issue on Django 5.2 LTS, and I'd like to propose either a backport or a documentation fix for the stable/5.2.x and stable/6.0.x branches.
Currently in 5.2 and 6.0, robust=True does the opposite of what it promises for partial callbacks: the except clause that is supposed to swallow the error raises an AttributeError of its own ('functools.partial' object has no attribute '__qualname__'), turning a handled error into an unhandled one.
from functools import partial from django.db import transaction def boom(**kwargs): raise OSError("broker is down") transaction.on_commit(partial(boom, pk=7), robust=True) # AttributeError: 'functools.partial' object has no attribute '__qualname__'
This is particularly painful because it stays invisible until a failure actually happens in production, and the official 5.2/6.0 documentation explicitly recommends this exact combination.
The docs for on_commit() recommend:
"Callbacks will not be passed any arguments, but you can bind them with
functools.partial()"
And for robust=True they promise:
"All errors derived from Python's Exception class are caught and logged..."
Because 5.2 is an LTS release supported until April 2028, readers following the documentation will continue to hit this crash. Therefore, I'd like to propose two options for the triage team to consider:
Option 1: Code Backport (Preferred)
Backport the 2-line fix from #36487 to stable/5.2.x and stable/6.0.x. I understand the strict backport policy for extended support LTS releases (security/data-loss only). However, because the current behavior crashes a natively documented feature and turns handled errors into unhandled exceptions, an exception might be warranted.
Option 2: Documentation Warning (Fallback)
If a code backport is rejected under the backport policy, I propose adding a warning block to the 5.2 and 6.0 documentation for on_commit. The warning would clarify that robust=True crashes with functools.partial in these versions, and suggest the workaround (using a closure or manually setting __qualname__ on the partial).
I am happy to prepare the patch for whichever option the team prefers!
Change History (2)
comment:1 by , 3 weeks ago
comment:2 by , 3 weeks ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
I know this is not the answer you are hoping for, but we prefer to stay very conservative with extended support versions. It will raise eyebrows if we start merging non data-loss/security fixes into extended support versions. Regarding the documentation idea, we prefer not to document bugs. This issue ticket should discoverable enough.
Going forward, the new annual release cycle in which every release is an LTS will ideally reduce the number of orgs hanging out on extended support versions. In other words, for future situations like this, you will be able to upgrade to the version with the fix with the comfort of that version also being an LTS.
I reproduced this using the same callback against the current
stable/5.2.x,stable/6.0.x, andmainbranches.Results:
stable/5.2.xraisesAttributeError: 'functools.partial' object has no attribute '__qualname__'.stable/6.0.xraises the sameAttributeError.maincatches and logs the callback exception as expected.The behavior dates back to
4a1150b41d, which originally added therobustargument, so this isn't a regression in Django 5.2 or 6.0. It was fixed onmainby7a2f35b1b7for #36487.Both Django 5.2 and 6.0 are currently in extended support, during which only security and data-loss fixes are eligible for backporting. Documentation fixes are generally backported only to the latest stable release, where this issue is already fixed.
Therefore, although the reported behavior is real, I don't believe either proposed change currently qualifies for a backport under the supported versions policy. A merger would need to decide whether the fact that the 6.0 backport was missed during its mainstream support period warrants an exception.
I will keep things as is until someone else check this out too for a different opinion.