Skip to content

Commit 4a1150b

Browse files
SirAbhi13felixxm
SirAbhi13
authored and
felixxm
committed
Fixed #33616 -- Allowed registering callbacks that can fail in transaction.on_commit().
Thanks David Wobrock and Mariusz Felisiak for reviews.
1 parent be63c78 commit 4a1150b

File tree

7 files changed

+140
-15
lines changed

7 files changed

+140
-15
lines changed

‎django/db/backends/base/base.py‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import _thread
22
import copy
33
import datetime
4+
import logging
45
import threading
56
import time
67
import warnings
@@ -26,6 +27,8 @@
2627
NO_DB_ALIAS = "__no_db__"
2728
RAN_DB_VERSION_CHECK = set()
2829

30+
logger = logging.getLogger("django.db.backends.base")
31+
2932

3033
# RemovedInDjango50Warning
3134
def timezone_constructor(tzname):
@@ -417,7 +420,9 @@ def savepoint_rollback(self, sid):
417420

418421
# Remove any callbacks registered while this savepoint was active.
419422
self.run_on_commit = [
420-
(sids, func) for (sids, func) in self.run_on_commit if sid not in sids
423+
(sids, func, robust)
424+
for (sids, func, robust) in self.run_on_commit
425+
if sid not in sids
421426
]
422427

423428
@async_unsafe
@@ -723,28 +728,49 @@ def schema_editor(self, *args, **kwargs):
723728
)
724729
return self.SchemaEditorClass(self, *args, **kwargs)
725730

726-
def on_commit(self, func):
731+
def on_commit(self, func, robust=False):
727732
if not callable(func):
728733
raise TypeError("on_commit()'s callback must be a callable.")
729734
if self.in_atomic_block:
730735
# Transaction in progress; save for execution on commit.
731-
self.run_on_commit.append((set(self.savepoint_ids), func))
736+
self.run_on_commit.append((set(self.savepoint_ids), func, robust))
732737
elif not self.get_autocommit():
733738
raise TransactionManagementError(
734739
"on_commit() cannot be used in manual transaction management"
735740
)
736741
else:
737742
# No transaction in progress and in autocommit mode; execute
738743
# immediately.
739-
func()
744+
if robust:
745+
try:
746+
func()
747+
except Exception as e:
748+
logger.error(
749+
f"Error calling {func.__qualname__} in on_commit() (%s).",
750+
e,
751+
exc_info=True,
752+
)
753+
else:
754+
func()
740755

741756
def run_and_clear_commit_hooks(self):
742757
self.validate_no_atomic_block()
743758
current_run_on_commit = self.run_on_commit
744759
self.run_on_commit = []
745760
while current_run_on_commit:
746-
sids, func = current_run_on_commit.pop(0)
747-
func()
761+
_, func, robust = current_run_on_commit.pop(0)
762+
if robust:
763+
try:
764+
func()
765+
except Exception as e:
766+
logger.error(
767+
f"Error calling {func.__qualname__} in on_commit() during "
768+
f"transaction (%s).",
769+
e,
770+
exc_info=True,
771+
)
772+
else:
773+
func()
748774

749775
@contextmanager
750776
def execute_wrapper(self, wrapper):

‎django/db/transaction.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,12 @@ def mark_for_rollback_on_error(using=None):
125125
raise
126126

127127

128-
def on_commit(func, using=None):
128+
def on_commit(func, using=None, robust=False):
129129
"""
130130
Register `func` to be called when the current transaction is committed.
131131
If the current transaction is rolled back, `func` will not be called.
132132
"""
133-
get_connection(using).on_commit(func)
133+
get_connection(using).on_commit(func, robust)
134134

135135

136136
#################################

‎django/test/testcases.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
from django.utils.version import PY310
6060
from django.views.static import serve
6161

62+
logger = logging.getLogger("django.test")
63+
6264
__all__ = (
6365
"TestCase",
6466
"TransactionTestCase",
@@ -1510,10 +1512,23 @@ def captureOnCommitCallbacks(cls, *, using=DEFAULT_DB_ALIAS, execute=False):
15101512
finally:
15111513
while True:
15121514
callback_count = len(connections[using].run_on_commit)
1513-
for _, callback in connections[using].run_on_commit[start_count:]:
1515+
for _, callback, robust in connections[using].run_on_commit[
1516+
start_count:
1517+
]:
15141518
callbacks.append(callback)
15151519
if execute:
1516-
callback()
1520+
if robust:
1521+
try:
1522+
callback()
1523+
except Exception as e:
1524+
logger.error(
1525+
f"Error calling {callback.__qualname__} in "
1526+
f"on_commit() (%s).",
1527+
e,
1528+
exc_info=True,
1529+
)
1530+
else:
1531+
callback()
15171532

15181533
if callback_count == len(connections[using].run_on_commit):
15191534
break

‎docs/releases/4.2.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,10 @@ Models
212212
* :ref:`Registering lookups <lookup-registration-api>` on
213213
:class:`~django.db.models.Field` instances is now supported.
214214

215+
* The new ``robust`` argument for :func:`~django.db.transaction.on_commit`
216+
allows performing actions that can fail after a database transaction is
217+
successfully committed.
218+
215219
Requests and Responses
216220
~~~~~~~~~~~~~~~~~~~~~~
217221

‎docs/topics/db/transactions.txt‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ include a `Celery`_ task, an email notification, or a cache invalidation.
297297
Django provides the :func:`on_commit` function to register callback functions
298298
that should be executed after a transaction is successfully committed:
299299

300-
.. function:: on_commit(func, using=None)
300+
.. function:: on_commit(func, using=None, robust=False)
301301

302302
Pass any function (that takes no arguments) to :func:`on_commit`::
303303

@@ -325,6 +325,15 @@ If that hypothetical database write is instead rolled back (typically when an
325325
unhandled exception is raised in an :func:`atomic` block), your function will
326326
be discarded and never called.
327327

328+
It's sometimes useful to register callback functions that can fail. Passing
329+
``robust=True`` allows the next functions to be executed even if the current
330+
function throws an exception. All errors derived from Python's ``Exception``
331+
class are caught and logged to the ``django.db.backends.base`` logger.
332+
333+
.. versionchanged:: 4.2
334+
335+
The ``robust`` argument was added.
336+
328337
Savepoints
329338
----------
330339

@@ -366,10 +375,14 @@ registered.
366375
Exception handling
367376
------------------
368377

369-
If one on-commit function within a given transaction raises an uncaught
370-
exception, no later registered functions in that same transaction will run.
371-
This is the same behavior as if you'd executed the functions sequentially
372-
yourself without :func:`on_commit`.
378+
If one on-commit function registered with ``robust=False`` within a given
379+
transaction raises an uncaught exception, no later registered functions in that
380+
same transaction will run. This is the same behavior as if you'd executed the
381+
functions sequentially yourself without :func:`on_commit`.
382+
383+
.. versionchanged:: 4.2
384+
385+
The ``robust`` argument was added.
373386

374387
Timing of execution
375388
-------------------

‎tests/test_utils/tests.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2285,6 +2285,32 @@ def branch_2():
22852285

22862286
self.assertEqual(callbacks, [branch_1, branch_2, leaf_3, leaf_1, leaf_2])
22872287

2288+
def test_execute_robust(self):
2289+
class MyException(Exception):
2290+
pass
2291+
2292+
def hook():
2293+
self.callback_called = True
2294+
raise MyException("robust callback")
2295+
2296+
with self.assertLogs("django.test", "ERROR") as cm:
2297+
with self.captureOnCommitCallbacks(execute=True) as callbacks:
2298+
transaction.on_commit(hook, robust=True)
2299+
2300+
self.assertEqual(len(callbacks), 1)
2301+
self.assertIs(self.callback_called, True)
2302+
2303+
log_record = cm.records[0]
2304+
self.assertEqual(
2305+
log_record.getMessage(),
2306+
"Error calling CaptureOnCommitCallbacksTests.test_execute_robust.<locals>."
2307+
"hook in on_commit() (robust callback).",
2308+
)
2309+
self.assertIsNotNone(log_record.exc_info)
2310+
raised_exception = log_record.exc_info[1]
2311+
self.assertIsInstance(raised_exception, MyException)
2312+
self.assertEqual(str(raised_exception), "robust callback")
2313+
22882314

22892315
class DisallowedDatabaseQueriesTests(SimpleTestCase):
22902316
def test_disallowed_database_connections(self):

‎tests/transaction_hooks/tests.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,47 @@ def test_executes_immediately_if_no_transaction(self):
4343
self.do(1)
4444
self.assertDone([1])
4545

46+
def test_robust_if_no_transaction(self):
47+
def robust_callback():
48+
raise ForcedError("robust callback")
49+
50+
with self.assertLogs("django.db.backends.base", "ERROR") as cm:
51+
transaction.on_commit(robust_callback, robust=True)
52+
self.do(1)
53+
54+
self.assertDone([1])
55+
log_record = cm.records[0]
56+
self.assertEqual(
57+
log_record.getMessage(),
58+
"Error calling TestConnectionOnCommit.test_robust_if_no_transaction."
59+
"<locals>.robust_callback in on_commit() (robust callback).",
60+
)
61+
self.assertIsNotNone(log_record.exc_info)
62+
raised_exception = log_record.exc_info[1]
63+
self.assertIsInstance(raised_exception, ForcedError)
64+
self.assertEqual(str(raised_exception), "robust callback")
65+
66+
def test_robust_transaction(self):
67+
def robust_callback():
68+
raise ForcedError("robust callback")
69+
70+
with self.assertLogs("django.db.backends", "ERROR") as cm:
71+
with transaction.atomic():
72+
transaction.on_commit(robust_callback, robust=True)
73+
self.do(1)
74+
75+
self.assertDone([1])
76+
log_record = cm.records[0]
77+
self.assertEqual(
78+
log_record.getMessage(),
79+
"Error calling TestConnectionOnCommit.test_robust_transaction.<locals>."
80+
"robust_callback in on_commit() during transaction (robust callback).",
81+
)
82+
self.assertIsNotNone(log_record.exc_info)
83+
raised_exception = log_record.exc_info[1]
84+
self.assertIsInstance(raised_exception, ForcedError)
85+
self.assertEqual(str(raised_exception), "robust callback")
86+
4687
def test_delays_execution_until_after_transaction_commit(self):
4788
with transaction.atomic():
4889
self.do(1)

0 commit comments

Comments
 (0)