Opened 13 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 , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 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 , 11 years ago
Cc: | added |
---|
comment:5 by , 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 , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 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 , 11 years ago
Has patch: | set |
---|
comment:9 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Sure, all value-added information we can bring might be useful for the poor debugging dev :-)