Code

Opened 2 years ago

Closed 2 years ago

#18449 closed Bug (duplicate)

Having an error raised during a transaction-decorated view will drop the error on the floor

Reported by: jstpierre@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: transaction
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider:

@commit_manually
def my_view():
    transaction.set_dirty() # or a DB call or something
    raise Exception("asdf")

As the transaction is dirty, when exiting the decorator, it will raise an exception because the transaction is dirty. Duh. Instead, if there is an exc_value set in the exiting function, Django should re-raise it immediately and not drop my exception on the floor.

Attachments (0)

Change History (4)

comment:1 Changed 2 years ago by aaugustin

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Keywords transaction added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

Here's the same problem shown in a shell:

>>> from django.db import transaction 
>>> with transaction.commit_manually():
...     transaction.set_dirty()
...     raise ValueError('doh!')
... 
Traceback (most recent call last):
  File "<console>", line 3, in <module>
  File "/Users/myk/Documents/dev/django-trunk/django/db/transaction.py", line 202, in __exit__
    self.exiting(exc_value, self.using)
  File "/Users/myk/Documents/dev/django-trunk/django/db/transaction.py", line 287, in exiting
    leave_transaction_management(using=using)
  File "/Users/myk/Documents/dev/django-trunk/django/db/transaction.py", line 51, in leave_transaction_management
    connection.leave_transaction_management()
  File "/Users/myk/Documents/dev/django-trunk/django/db/backends/__init__.py", line 123, in leave_transaction_management
    "Transaction managed block ended with pending COMMIT/ROLLBACK")
TransactionManagementError: Transaction managed block ended with pending COMMIT/ROLLBACK

At first sight, it would be more logical (and easier to debug) if the original exception wasn't swallowed. I'd like if someone familiar with transaction management could confirm.

comment:2 Changed 2 years ago by akaariai

  • Triage Stage changed from Accepted to Design decision needed

The way commit_manually works is that you must either commit or rollback the transaction. This is explicitly documented. So, the given TransactionManagementError is more like a hint saying that you are using the commit_manually decorator improperly.

I don't think this should be changed in a way where the original exception is risen and no hint is given. This could be a source of transaction leak. There might be some point of changing the decorator to do automatic rollback on exceptions. This would be a change from currently documented behavior.

As currently documented, the reported problem is an user error. If the currently documented behavior should be changed needs a design decision.

comment:3 Changed 2 years ago by jstpierre@…

Since I filed this bug, I found a duplicate somewhere with a patch. I cannot find it again for the life of me, sorry.

comment:4 Changed 2 years ago by jstpierre@…

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #6623

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.