Opened 11 years ago

Closed 11 years ago

#19920 closed Bug (fixed)

DatabaseError exceptions in _commit, _rollback, etc.

Reported by: Aymeric Augustin Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

#14223 unified the IntegrityError exceptions raised in _commit.

For consistency, Django should also unify DatabaseError exceptions raised in _commit and _rollback.

To be fully consistent, Django should also wrap exceptions in savepoint-related methods.

I'm going to need this for error handling in persistent connections.


With the current approach this is going to result in a lot of duplicated code:

            try:
                # do stuff
            except Database.IntegrityError as e:
                six.reraise(utils.IntegrityError, utils.IntegrityError(*tuple(e.args)), sys.exc_info()[2])
            except Database.DatabaseError as e:
                six.reraise(utils.DatabaseError, utils.DatabaseError(*tuple(e.args)), sys.exc_info()[2])

A better solution would be to abstract this pattern into a decorator.


A less magical alternative is to make the exceptions attributes of the database wrapper. This would allow code such as:

for conn in connections.all():
    try:
        conn.rollback()
    except conn.DatabaseError:
        pass

This solution seems more robust to me.

It's suggested as an extension by PEP249: http://www.python.org/dev/peps/pep-0249/#connection-error

It's more backwards compatible with the current code because it doesn't require changing the type of some exceptions.

It avoids problems with catching the "wrong" DatabaseError (Django's vs. the database adapter's). This is an easy mistake, and database exceptions are very hard to test — one cannot shut down the database during tests!

I'm mostly logging this alternative for posterity, because Django made a different choice long ago, and reversing it would be complicated — all the more since a non PEP-249 exception was introduced in the hierarchy: ProtectedError, see https://docs.djangoproject.com/en/dev/ref/exceptions/#transaction-exceptions

Change History (2)

comment:2 by Aymeric Augustin <aymeric.augustin@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 59a352087591a26023412cbcb830cd1d34fc9b99:

Refactored database exceptions wrapping.

Squashed commit of the following:

commit 2181d833ed1a2e422494738dcef311164c4e097e
Author: Aymeric Augustin <aymeric.augustin@…>
Date: Wed Feb 27 14:28:39 2013 +0100

Fixed #15901 -- Wrapped all PEP-249 exceptions.

commit 5476a5d93c19aa2f928c497d39ce6e33f52694e2
Author: Aymeric Augustin <aymeric.augustin@…>
Date: Tue Feb 26 17:26:52 2013 +0100

Added PEP 3134 exception chaining.

Thanks Jacob Kaplan-Moss for the suggestion.

commit 9365fad0a650328002fb424457d675a273c95802
Author: Aymeric Augustin <aymeric.augustin@…>
Date: Tue Feb 26 17:13:49 2013 +0100

Improved API for wrapping database errors.

Thanks Alex Gaynor for the proposal.

commit 1b463b765f2826f73a8d9266795cd5da4f8d5e9e
Author: Aymeric Augustin <aymeric.augustin@…>
Date: Tue Feb 26 15:00:39 2013 +0100

Removed redundant exception wrapping.

This is now taken care of by the cursor wrapper.

commit 524bc7345a724bf526bdd2dd1bcf5ede67d6bb5c
Author: Aymeric Augustin <aymeric.augustin@…>
Date: Tue Feb 26 14:55:10 2013 +0100

Wrapped database exceptions in the base backend.

This covers the most common PEP-249 APIs:

  • Connection APIs: close(), commit(), rollback(), cursor()
  • Cursor APIs: callproc(), close(), execute(), executemany(), fetchone(), fetchmany(), fetchall(), nextset().

Fixed #19920.

commit a66746bb5f0839f35543222787fce3b6a0d0a3ea
Author: Aymeric Augustin <aymeric.augustin@…>
Date: Tue Feb 26 14:53:34 2013 +0100

Added a wrap_database_exception context manager and decorator.

It re-throws backend-specific exceptions using Django's common wrappers.

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