Re: "unexpected EOF" messages

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: "unexpected EOF" messages
Date: 2012-05-03 17:36:16
Message-ID: CABUevExSj1+BpSQTC9k998Q7sDBG=rQYhJy7B=k+MguSL8cE5A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 3, 2012 at 7:21 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Thu, May 3, 2012 at 7:09 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> On Thu, May 3, 2012 at 5:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> I could support that with one tweak: it's only DEBUG1 if you don't
>>>> have an open transaction.  Dropping the connection while in a
>>>> transaction *is* an application bug; I don't care how lazy the app
>>>> programmer is feeling.
>>
>>> I agree - that would certainly be a good fix for this one. One
>>> question is do we want something like this:
>>
>>> -                   ereport(COMMERROR,
>>> +                   ereport(IsTransactionState() ? COMMERROR : DEBUG1,
>>>                             (errcode(ERRCODE_PROTOCOL_VIOLATION),
>>>                              errmsg("unexpected EOF on client connection")));
>>
>>> or do we want to make the text of the error message different as well,
>>> saying something like "unexpected EOF on client connection with an
>>> open transaction"?
>>
>> I'd vote for different texts and different SQLSTATEs too, per other
>> discussion.  (I think we'd decided that ERRCODE_PROTOCOL_VIOLATION
>> was a bad choice anyway.)
>>
>> Also, I'm afraid that the above patch probably doesn't work as-is;
>> won't elog.c try to send the DEBUG1 message to the client?  I think
>> you'll need some additional code to shut down error message output
>> first.  Resetting whereToSendOutput is probably sufficient.
>
> Yeah, I didn't go as far as testing it - there's also more than one
> spot where we log it... I'll cook up a patch.

Heh - we already used ERRCODE_CONNECTION_FAILURE on the errors in
copy.c. Since COPY can only happen when there is a transaction
(right?), I just changed those error messages for consistency.

This patch works through my testing - can anyone spot a hole in it still?

The next question is - of course - whether we can sneak this in before beta...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Attachment Content-Type Size
ereport_eof.patch application/octet-stream 4.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2012-05-03 17:44:05 Re: "unexpected EOF" messages
Previous Message Tom Lane 2012-05-03 17:28:43 Re: Advisory locks seem rather broken