[Django] #17601: Error code from database exception should not be lost during exception handling (psycopg2)

Django

unread,

Jan 27, 2012, 5:38:12 AM1/27/12

to django-...@googlegroups.com

#17601: Error code from database exception should not be lost during exception
handling (psycopg2)
----------------------------------------------+--------------------
Reporter: zimnyx | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: SVN
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
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
[http://initd.org/psycopg/docs/module.html#psycopg2.Error.pgcode "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.[[BR]]
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".

--
Ticket URL: <https://code.djangoproject.com/ticket/17601>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Jan 27, 2012, 2:08:06 PM1/27/12

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: SVN
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):

* needs_docs: => 0
* needs_better_patch: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted

Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:1>

Django

unread,

Jun 3, 2013, 1:46:35 PM6/3/13

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

Comment (by jaylett):

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):
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:2>

Django

unread,

Jun 3, 2013, 1:49:46 PM6/3/13

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: nobody

Changes (by jaylett):

* cc: james@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:3>

Django

unread,

Jun 3, 2013, 2:34:51 PM6/3/13

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: nobody

Comment (by aaugustin):

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

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:4>

Django

unread,

Jun 4, 2013, 6:46:53 AM6/4/13

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: nobody

Comment (by jaylett):

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.)

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:5>

Django

unread,

Jun 4, 2013, 7:36:21 AM6/4/13

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: jaylett
Type: | Status: assigned

Changes (by jaylett):

* owner: nobody => jaylett
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:6>

Django

unread,

Jun 4, 2013, 7:40:21 AM6/4/13

to django-...@googlegroups.com

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

Comment (by jaylett):

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

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:7>

Django

unread,

Jun 4, 2013, 7:40:53 AM6/4/13

to django-...@googlegroups.com

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

Has patch: 1 | UI/UX: 0

Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by jaylett):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:8>

Django

unread,

Jun 4, 2013, 8:28:31 AM6/4/13

to django-...@googlegroups.com

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

-------------------------------------+-------------------------------------
Reporter: zimnyx | Owner: jaylett

Type: | Status: closed
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed

Changes (by James Aylett <james@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"54485557855d58d9a5027511025d5ab22f721c6d"]:
{{{
#!CommitTicketReference repository=""
revision="54485557855d58d9a5027511025d5ab22f721c6d"
Fixed #17601 -- expose underlying db exceptions under py2

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

--
Ticket URL: <https://code.djangoproject.com/ticket/17601#comment:9>

Reply all

Reply to author

Forward