Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#28091 closed Bug (fixed)

Hiding 'cursor does not exist' error doesn't work

Reported by: Claude Paroz Owned by: François Freitag
Component: Database layer (models, ORM) Version: 1.11
Severity: Release blocker Keywords:
Cc: mail@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In commit [6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf], Django tries to mask the 'cursor does not exist' exception because it masks the real database exception. Unfortunately, the commit is not fulfilling its promise, as the raise command just after the pass, will in fact raise the second exception (the one we are trying to mask) not the original one.

Change History (8)

comment:1 by Claude Paroz, 8 years ago

Unfortunately, until now, I wasn't able to reproduce the error with the Django test suite. In my project, I had a new model which was not yet migrated, but even with that scenario, I was not able to reproduce on Django. Any help welcome, as I think a test would be really helpful here.

comment:2 by François Freitag, 8 years ago

Owner: changed from nobody to François Freitag
Status: newassigned

comment:3 by François Freitag, 8 years ago

Hi Claude,

I have a test that's failing before [6b6be692fcd102436c7abef1d7b3fa1d37ad4bdf] and passes after this commit.
Were you able to gather more information about your failure?

comment:4 by Claude Paroz, 8 years ago

I'm getting that:

======================================================================
ERROR: test_cursor_close_failure_does_not_mask_original_exception (backends.tests.BackendTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/claude/checkouts/django-git/tests/backends/tests.py", line 695, in test_cursor_close_failure_does_not_mask_original_exception
    mock_cursor.execute = mock.MagicMock(side_effect=raise_original_exception)
AttributeError: 'psycopg2.extensions.cursor' object attribute 'execute' is read-only

comment:5 by François Freitag, 8 years ago

I updated the test because my cursor creation was flawed. I should have used directly connection.chunked_cursor() instead of trying to be smart and use create_cursor().

Fixing this error made the test fail with:

FAIL: test_cursor_close_failure_does_not_mask_original_exception (backends.tests.BackendTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/home/freitafr/dev/django/tests/backends/tests.py", line 700, in test_cursor_close_failure_does_not_mask_original_exception
    list(persons)
  File "/usr/lib64/python2.7/contextlib.py", line 35, in __exit__
    self.gen.throw(type, value, traceback)
  File "/home/freitafr/dev/django/django/test/testcases.py", line 615, in _assert_raises_message_cm
    self.assertIn(expected_message, str(cm.exception))
  File "/usr/lib64/python2.7/unittest/case.py", line 803, in assertIn
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib64/python2.7/unittest/case.py", line 410, in fail
    raise self.failureException(msg)
AssertionError: u'Real exception raised by the database on cursor.execute' not found in 'Error when attempting to close the cursor, would mask the actual exception'

I opened #8371 with a suggested way to fix this issue.

Version 1, edited 8 years ago by François Freitag (previous) (next) (diff)

comment:6 by Claude Paroz <claude@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 56746fb2:

[1.11.x] Fixed #28091 -- Re-raised original exception when closing cursor cleanup fails

comment:7 by Tim Graham <timograham@…>, 8 years ago

In c9d933ba:

[1.11.x] Refs #28091 -- Fixed typo and rephrased 1.11.1 release note.

comment:8 by Tim Graham <timograham@…>, 8 years ago

In 8b767986:

Refs #28091 -- Forwardported 1.11.1 release note.

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