Re: [PATCH v2] fetch-pack: try harder to read an ERR packet

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> Hmm, if the connection gets severed just before the ERR packet the
>> other side has written, we will see "Broken pipe" if we write
>> "done", and no amount of "try to read to collect as much what they
>> said as possible" would help.  If you are lucky and the connection
>> is broken after the ERR reaches on this side, such an "extra effort"
>> may help, but is it really worth the effort?  It is not clear to me
>> if the extra complexity, one more API function people need to learn,
>> and the need to think which one to use every time they want to say
>> write_in_full(), are justifiable.
>
> I think the "lucky" case happens pretty routinely. The situation we're
> trying to catch here is that server does:
>
>    packet_write("ERR I don't like your request for some reason");
>    die("exiting");

OK, if we assume that the communication route is never flaky and
everything writtten will go through before TCP shutdown that would
happen when die() kills the process (or like test environment that
most communication goes locally between two processes), sure, it may
look common enough to be worth "fixing".  I simply did not realize
that was the shared assumption behind this patch before I went back
to the original discussion that was about a racy test.

As long as an extra read on the side that just got a write error
won't throw us into a deadlock, I think I am OK, but I am still not
sure if the code complexity to have two write_in_full() is worth it.

If the mechanism to do this were limited to the packet IO layer, it
may be more palatable, though.