Opened 12 years ago

Closed 11 years ago

#17601 closed Cleanup/optimization (fixed)

Error code from database exception should not be lost during exception handling (psycopg2)

Reported by: Piotr Czachur Owned by: James Aylett
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: james@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here is a fragment from source:django/trunk/django/db/backends/postgresql_psycopg2/base.py?rev=17244#L50

    def execute(self, query, args=None):
        try:
            return self.cursor.execute(query, args)
        except Database.IntegrityError, e:
            raise utils.IntegrityError, utils.IntegrityError(*tuple(e)), sys.exc_info()[2]
        except Database.DatabaseError, e:
            raise utils.DatabaseError, utils.DatabaseError(*tuple(e)), sys.exc_info()[2]

In case of Psycopg2 and database exceptions, error code is set as psycopg2 exception propery called pgcode instead of being passed as exception argument, so this way we're loosing info about "pgcode" which is valuable. Handling database exception by parsing error message is a bit awkward.

I'm not sure if other backends are affected by this issue, but I can investigate and provide a patch if we agree on this issue.

P.S.
I've just filled a bug in this subject against Psycopg2: http://psycopg.lighthouseapp.com/projects/62710-psycopg/tickets/96-pgcode-should-be-present-in-exceptionargs
If they fix it, this ticket will become "worksforme".

Change History (9)

comment:1 by Claude Paroz, 12 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Sure, all value-added information we can bring might be useful for the poor debugging dev :-)

comment:2 by James Aylett, 11 years ago

The bug filed against psycopg2 was closed, rightly. I think what is needed (within Django) is a way of getting at the original exception given the common wrapper, so that (in this case) it's possible to access pgcode and particularly diag from the psycopg2 exception. In python 3 this is available via __cause__ (AIUI; I haven't actually used py3 in anger). Could we just set __cause__ whether we're running under py3 or not? Something like (at [c36b75c814f068fcb722d46bd5e71cbaddf9bf2d]):

--- a/django/db/utils.py
+++ b/django/db/utils.py
@@ -91,8 +91,7 @@ class DatabaseErrorWrapper(object):
                 except AttributeError:
                     args = (exc_value,)
                 dj_exc_value = dj_exc_type(*args)
-                if six.PY3:
-                    dj_exc_value.__cause__ = exc_value
+                dj_exc_value.__cause__ = exc_value
                 # Only set the 'errors_occurred' flag for errors that may make
                 # the connection unusable.
                 if dj_exc_type not in (DataError, IntegrityError):

comment:3 by James Aylett, 11 years ago

Cc: james@… added

comment:4 by Aymeric Augustin, 11 years ago

I suppose setting cause unconditionally can't hurt...

comment:5 by James Aylett, 11 years ago

I can't think of a way it would, since it doesn't exist at the moment. (It's possible that poorly-written code might use it to assume it's running under py3, but that seems a stretch.)

I'm not quite sure where we'd want to document this; probably a small note in ref/exceptions?. (There are no tests that use __cause__ as far as I can tell, so I don't know if we need an explicit test case for this.)

comment:6 by James Aylett, 11 years ago

Owner: changed from nobody to James Aylett
Status: newassigned

comment:7 by James Aylett, 11 years ago

Pull request 1241 (https://github.com/django/django/pull/1241) rather than messing around with tiny patches in comments.

comment:8 by James Aylett, 11 years ago

Has patch: set

comment:9 by James Aylett <james@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 54485557855d58d9a5027511025d5ab22f721c6d:

Fixed #17601 -- expose underlying db exceptions under py2

Use cause to expose the underlying database exceptions even
under python 2.

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