From e3c6b464160a7a38cbef288c53c6ebe35e4b4698 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Sep 2014 06:46:39 +0100 Subject: [PATCH] Don't try to close the server cursor in error state `close()` is implicitly called by `__exit__()`, so an exit on error would run a query on a inerr connection, causing another exception hiding the original one. The fix is on `close()`, not on `__exit__()`, because the semantic of the latter is simply to call the former. Closes #262. --- NEWS | 7 +++++++ psycopg/cursor_type.c | 20 +++++++++++++++++--- tests/test_with.py | 12 ++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 3062c8ed..639759ee 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,13 @@ Current release --------------- +What's new in psycopg 2.5.5 +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- Named cursors used as context manager don't swallow the exception on exit + (:ticket:`#262`). + + What's new in psycopg 2.5.4 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index a7591ee9..67c51723 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -57,10 +57,24 @@ psyco_curs_close(cursorObject *self) if (self->name != NULL) { char buffer[128]; + PGTransactionStatusType status; - EXC_IF_NO_MARK(self); - PyOS_snprintf(buffer, 127, "CLOSE \"%s\"", self->name); - if (pq_execute(self, buffer, 0, 0, 1) == -1) return NULL; + if (self->conn) { + status = PQtransactionStatus(self->conn->pgconn); + } + else { + status = PQTRANS_UNKNOWN; + } + + if (!(status == PQTRANS_UNKNOWN || status == PQTRANS_INERROR)) { + EXC_IF_NO_MARK(self); + PyOS_snprintf(buffer, 127, "CLOSE \"%s\"", self->name); + if (pq_execute(self, buffer, 0, 0, 1) == -1) return NULL; + } + else { + Dprintf("skipping named curs close because tx status %d", + (int)status); + } } self->closed = 1; diff --git a/tests/test_with.py b/tests/test_with.py index d39016c6..13e4d7ef 100755 --- a/tests/test_with.py +++ b/tests/test_with.py @@ -200,6 +200,18 @@ class WithCursorTestCase(WithTestCase): self.assert_(curs.closed) self.assert_(closes) + def test_exception_swallow(self): + # bug #262: __exit__ calls cur.close() that hides the exception + # with another error. + try: + with self.conn as conn: + with conn.cursor('named') as cur: + cur.execute("select 1/0") + except psycopg2.DataError, e: + self.assertEqual(e.pgcode, '22012') + else: + self.fail("where is my exception?") + def test_suite(): return unittest.TestLoader().loadTestsFromName(__name__)