Atomic masks fatal database errors and instead propagates "connection already closed"
|Reported by:||intgr||Owned by:||aaugustin|
|Component:||Database layer (models, ORM)||Version:||1.6-beta-1|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
I noticed this problem with the new Atomic code in Django 1.6: when the PostgreSQL server raises a FATAL error (meaning that the server closes the connection after the error), then the Atomic block will swallow the original error and propagates a meaningless "InterfaceError: connection already closed" instead. I don't know whether this also affects other database backends.
I find that it's quite annoying to debug problems when the original cause of an error is hidden by subsequent error recovery code. It's not a new problem with Atomic, I believe commit_on_success (and Christophe Pettus's xact) had the same problem.
The easiest way to reproduce this is to terminate your own connection. (In PostgreSQL 9.1 and earlier you need to be superuser to do this):
from django.db import connection from django.db.transaction import atomic with atomic(): cur = connection.cursor() cur.execute("select pg_terminate_backend(pg_backend_pid())") cur.fetchall()
With current Django 1.6 this raises an error like:
Traceback (most recent call last): File "term1.py", line 7, in <module> cur.fetchall() File "/home/marti/src/django/django/db/transaction.py", line 351, in __exit__ connection.set_autocommit(True) File "/home/marti/src/django/django/db/backends/__init__.py", line 335, in set_autocommit self._set_autocommit(autocommit) File "/home/marti/src/django/django/db/backends/postgresql_psycopg2/base.py", line 184, in _set_autocommit self.connection.autocommit = autocommit psycopg2.InterfaceError: connection already closed
It tells you that the connection is closed, but not why. And there's a second problem too: a psycopg2 exception is propagated into user code, without using Django's exception wrappers.
To solve this problem, I submitted pull request 1696: https://github.com/django/django/issues/1696
- Changed BaseDatabaseWrapper.set_autocommit to wrap backend-specific errors.
- Wrapped Atomic.__exit__ in another layer of try/except and re-raising the original exception in case of an InterfaceError.
Now the original exception is properly raised:
django.db.utils.OperationalError: terminating connection due to administrator command
I can also write unit tests if someone can help me figure out how to test this (the example I provided above requires PostgreSQL, with either superuser privs or version 9.2+).
Ran all 'transactions' and 'db_backends' tests on these changes with both SQLite (OK (skipped=42)) and psycopg2 (OK (skipped=1))
Change History (13)
comment:1 Changed 21 months ago by aaugustin
- Needs documentation unset
- Needs tests unset
- Owner changed from nobody to aaugustin
- Patch needs improvement set
- Status changed from new to assigned
- Triage Stage changed from Unreviewed to Accepted
- Type changed from Uncategorized to Cleanup/optimization
comment:9 Changed 15 months ago by Aymeric Augustin <aymeric.augustin@…>
- Resolution set to fixed
- Status changed from assigned to closed