Catch psycopg disconnect error

[Former user]

(original reporter: elic) I've found a rare libpq disconnect error which isn't being caught by the psycopg2 dialect:

When a TCP Reset happens on an existing connection, psycopg raises DatabaseError("server closed the connection unexpectedly"). PGDialect_psycopg2.is_disconnect() doesn't have a check for this message, which causes Connection._handle_dbapi_error() to throw a spurious InterfaceError, as well as causes it to fail to .invalidate() the connection (which subsequently causes things like Session.rollback() to fail as well).

Attached is a script which reproduces this behaviour (it requires linux iptables & sudo in order to force the tcp reset).

Also attached is a patch adds support for this error to PGDialect_psycopg2.is_disconnect()

[Former user]

(original author: elic) script reproducing bug

[Former user]

(original author: elic) patch to catch error

[Mike Bayer]

OK, that message you can see we're already looking for almost the same thing twice above it, so I propose we just open it up to look for all of these messages straight down for all exceptions. Otherwise, each time psycopg2/libpq makes some subtle change, the same message moves from ProgrammingError to InterfaceError etc. I've used postgresql for many years and these messages pretty much mean the connection died.

Let me know if what I did in 00c163bfb13d273e61dcb7ec78ac96338c916de7 works for you, the tests we have for "disconnect" pass with this and the test you attached should work also.

[Former user]

(original author: elic) Works fine for me and my test case, and definitely much more resilient to changes in psycopg's exception type mapping. There's just one rare (perhaps pathological) border case I noticed yesterday after submitting my original patch:

Checks for {{{xxx in str(e)}}} could result in a false positive if the substring appears within the error message as part of a column name or other statement-derived text. For example, the error messages thrown by {{{SELECT 'cursor closed'::text+1}}}, {{{SELECT "cursor closed"}}}, and {{{EXECUTE "cursor closed"}}} will now get treated as a disconnection. (Actually, I think some of these cases could slip through before as well).

Attached is a patch to 00c163bfb13d273e61dcb7ec78ac96338c916de7 (named exclude_programming_errors.patch) which attempts to mitigate that border case...

First, it strips all but the first line from {{{str(e)}}}. After examining the libpq source, I'm 99% certain all the substrings it's looking for occur on the first line. This should take care of errors where the sql content is embedded in subsequent lines, such as for {{{SELECT 'cursor closed'::text+1}}}, which results in {{{operator does not exist: text + integer\nLINE 1: select 'cursor closed'::text+1;\n}}}.

Second, when it finds a match to one of the substrings, it checks if there's a double-quote anywhere before the match location, to exclude cases where the substring occurs as part of some quoted statement text. This should take care of messages which include a quoted identifier on the first line, such as for {{{SELECT "cursor closed"}}}, which results in {{{column "cursor closed" does not exist\n}}}.

Sidenote: I initially tried coding a fix by excluding certain types (DataError, IntegrityError, and ProgrammingError) from the test, but after looking closer at psycopg's exception_from_sqlstate(), I realized that wasn't enough: for example, {{{EXECUTE "cursor closed"}}} results a message similar to the 2nd case, but it's an OperationalError, not a ProgrammingError.

[Former user]

(original author: elic) exclude statement text from is_disconnect() checks

[Mike Bayer]

I had thought of that also but wasn't going to bother :). But sure, this should help, in 177ad548c458a9f1b8d43378fa8f16e349f80ac3. Forgive my internet stalking to get your real name for the changelog.

[Former user]

(original author: elic) No problem! Glad it helps.

[Mike Bayer]

Removing milestone: 0.8.xx (automated comment)