Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it ok to catch and reraise an exception inside Django transaction.atomic()?

Django's docs say this about transaction.atomic() and exceptions:

https://docs.djangoproject.com/en/1.10/topics/db/transactions/#django.db.transaction.atomic

Avoid catching exceptions inside atomic!

When exiting an atomic block, Django looks at whether it’s exited normally or with an exception to determine whether to commit or roll back. If you catch and handle exceptions inside an atomic block, you may hide from Django the fact that a problem has happened. This can result in unexpected behavior.

This is mostly a concern for DatabaseError and its subclasses such as IntegrityError. After such an error, the transaction is broken and Django will perform a rollback at the end of the atomic block. If you attempt to run database queries before the rollback happens, Django will raise a TransactionManagementError. You may also encounter this behavior when an ORM-related signal handler raises an exception.

The correct way to catch database errors is around an atomic block as shown above. If necessary, add an extra atomic block for this purpose. This pattern has another advantage: it delimits explicitly which operations will be rolled back if an exception occurs.

If you catch exceptions raised by raw SQL queries, Django’s behavior is unspecified and database-dependent.

Is it okay to do this or does this cause "unexpected behavior"?

with transaction.atomic():
    # something
    try:
        # something
    except:
        logger.exception("Report error here.")
        raise
    
like image 665
Scott Stafford Avatar asked Mar 03 '17 19:03

Scott Stafford


2 Answers

Based on the docs, I would ensure that the correct Exception gets re-raised, others errors you might handle independently. For django it is only necessary that it gets notified regarding things that went wrong when talking to the database.

with transaction.atomic():
    # something
    try:
        # something
    except DatabaseError as db_err:
        logger.exception("Report error here.")
        raise db_err
    except Exception:
        # do something else
        # no need to reraise
        # as long as you handle it properly and db integrity is guaranteed
like image 113
dahrens Avatar answered Oct 24 '22 01:10

dahrens


Is it okay to do this or does this cause "unexpected behavior"?

with transaction.atomic():
    # something
    try:
        # something
    except:
        logger.exception("Report error here.")
        raise

Except for the bare except clause (you want at least except Exception:), this is ok, assuming your logger doesn't touch the database (which would be a very bad idea anyway) and the logger call doesn't raise another exception (in which case I have no idea what will actually happen).

But you'd get the same result inverting the transaction.atomic() block and the try/except, ie:

try:
    with transaction.atomic():
        # something
        # something
except Exception:
    logger.exception("Report error here.")
    raise
like image 43
bruno desthuilliers Avatar answered Oct 24 '22 02:10

bruno desthuilliers