Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#11900 closed (fixed)

commit_on_success: Handle Exception in commit()

Reported by: guettli Owned by: gabrielhurley
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: gabrielhurley Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you get an exception during the commit done by the commit_on_success decorator, you get a meaningless error message:

django.db.transaction.TransactionManagementError: Transaction managed block ended with pending COMMIT/ROLLBACK

With this patch you see the real exception.

Can someone please write a unittest for this?

Attachments (2)

transaction.py.patch (483 bytes) - added by guettli 6 years ago.
11900_patch_and_tests.diff (1.9 KB) - added by gabrielhurley 5 years ago.
Updates patch to work w/ MultiDB and adds tests

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by guettli

comment:1 Changed 6 years ago by guettli

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.1 to 1.0

comment:2 Changed 5 years ago by russellm

  • Has patch set
  • milestone set to 1.2
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 5 years ago by gabrielhurley

  • Cc gabrielhurley added
  • Owner changed from nobody to gabrielhurley
  • Status changed from new to assigned

The reason for the problem is clear enough: The error raised in the finally clause overrides the original error.

However, I tried for about 2 hours to reproduce the reported bug with no success.

guettli, can you some information about the code you're using that triggers the bug?

comment:4 Changed 5 years ago by guettli

Hi Gabriel,

thank you for your question. This script results in

Traceback (most recent call last):
  File "djangotest_tbz/script.py", line 10, in <module>
    main()
  File "/home/djangotest_tbz_d/django/db/transaction.py", line 310, in _commit_on_success
    leave_transaction_management(using=db)
  File "/home/djangotest_tbz_d/django/db/transaction.py", line 87, in leave_transaction_management
    raise TransactionManagementError("Transaction managed block ended with pending COMMIT/ROLLBACK")
django.db.transaction.TransactionManagementError: Transaction managed block ended with pending COMMIT/ROLLBACK

I just tried it with latest trunk ([12761]). Postgres (psycopg2) evaluates Foreign-Key
Constraints lazy (deferrable). If you break this constraint, you get an exception
during commit. But you don't see the real problem.

from django.db import transaction, connection

@transaction.commit_on_success
def main():
    cursor=connection.cursor()
    cursor.execute('INSERT INTO auth_user_groups VALUES (DEFAULT, 10000000, 1000000);')
    transaction.set_dirty()

if __name__=='__main__':
    main()

Changed 5 years ago by gabrielhurley

Updates patch to work w/ MultiDB and adds tests

comment:5 Changed 5 years ago by gabrielhurley

  • Needs tests unset

This specific bug seems to be backend-specific (it seems to be completely impossible to trigger it using SQLite3). I did manage to duplicate it, test it, and patch it using a server running psycopg2. The general principle of the fix seems universally applicable, though.

The patch I added has both the requested test and an updated version of the original patch that works with the new MultiDB in 1.2. Hopefully it's good to go now.

comment:6 Changed 5 years ago by guettli

Your patch is simple, contains a test and works for me. Thank you. Who can review and commit it?

comment:7 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [12764]) Fixed #11900 -- Corrected an edge case of transaction handling in the commit_on_success decorator. Thanks to guettli for the report, and Gabriel Hurley for the initial test case.

comment:8 Changed 5 years ago by guettli

  • Cc hv@… removed

comment:9 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top