Opened 4 months ago
Closed 4 months ago
#35618 closed Uncategorized (duplicate)
response.close() in a TestCase prematurely closes PostgreSQL connection and leads to psycopg2.InterfaceError
Reported by: | Anders Kaseorg | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Carlton Gibson, Jon Janzen | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If I call response.close()
on an HTTP response from the test client within a django.test.TestCase
, Django incorrectly closes the active PostgreSQL connection, causing future test requests to fail with psycopg2.InterfaceError: connection already closed
.
This happens because HttpResponseBase.close
sends signals.request_finished
, which is handled by django.db.close_old_connections
, which calls BaseDatabaseWrapper.close_if_unusable_or_obsolete
, which observes self.get_autocommit() != self.settings_dict["AUTOCOMMIT"]
(False != True
) and closes the connection.
Any connection that’s within a transaction (such as the one opened by TestCase
) will have get_autocommit() == False
. It seems very wrong that the finishing of any request automatically triggers all connections in transactions to be immediately closed.
Minimal complete test project: https://gist.github.com/andersk/b4da5a106995b4c525595ea204675ee0
$ ./manage.py test Found 1 test(s). Creating test database for alias 'default'... System check identified no issues (0 silenced). E ====================================================================== ERROR: test (tests.MyTest.test) ---------------------------------------------------------------------- Traceback (most recent call last): File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 294, in _cursor return self._prepare_cursor(self.create_cursor(name)) ^^^^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/utils/asyncio.py", line 26, in inner return func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/postgresql/base.py", line 332, in create_cursor cursor = self.connection.cursor() ^^^^^^^^^^^^^^^^^^^^^^^^ psycopg2.InterfaceError: connection already closed The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/tmp/close-test/tests.py", line 8, in test response = self.client.get("/two") ^^^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 1049, in get response = super().get(path, data=data, secure=secure, headers=headers, **extra) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 465, in get return self.generic( ^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 617, in generic return self.request(**r) ^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 1013, in request self.check_exception(response) File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/test/client.py", line 743, in check_exception raise exc_value File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/core/handlers/exception.py", line 55, in inner response = get_response(request) ^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/core/handlers/base.py", line 197, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/my_urls.py", line 7, in view with connection.cursor(): ^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/utils/asyncio.py", line 26, in inner return func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 316, in cursor return self._cursor() ^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 293, in _cursor with self.wrap_database_errors: File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/utils.py", line 91, in __exit__ raise dj_exc_value.with_traceback(traceback) from exc_value File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/base/base.py", line 294, in _cursor return self._prepare_cursor(self.create_cursor(name)) ^^^^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/utils/asyncio.py", line 26, in inner return func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^ File "/tmp/close-test/.direnv/python-3.12/lib/python3.12/site-packages/django/db/backends/postgresql/base.py", line 332, in create_cursor cursor = self.connection.cursor() ^^^^^^^^^^^^^^^^^^^^^^^^ django.db.utils.InterfaceError: connection already closed ---------------------------------------------------------------------- Ran 1 test in 0.018s FAILED (errors=1) Destroying test database for alias 'default'...
Change History (7)
comment:2 by , 4 months ago
The streaming case is the one that motivated me to call response.close
in the first place. Currently there’s no way to get closing_iterator_wrapper
to close the response itself without first consuming the entire stream, and leaving it unclosed results in various ResourceWarning
s when warnings are enabled.
comment:3 by , 4 months ago
Cc: | added |
---|
We had a similar issue in Channels, which we've addressed by (similarly) disabling the close_old_connections handler during tests.
We have this down as related to #30448.
comment:4 by , 4 months ago
Has patch: | set |
---|
comment:6 by , 4 months ago
Should we close as duplicate of #30448, this seems like the exact same problem.
comment:7 by , 4 months ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Yes, agreed. Duplicate of #30448.
In order to prevent this problem from happening under normal circumstances
django.test.Client
unregisters this signal handler before callingrequest.close
.I feel like if you're testing something as low level as
HttpRequest
closing in the context of database connection handling you should be usingClientHandler
?I guess that we could possibly make
request.close
idempotent given what's returned bytest.Client
should already be closed?