Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack

From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Max Kirillov <max@max630.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Jeff King <peff@peff.net>,
	Florian Manschwetus <manschwetus@cs-software-gmbh.de>,
	Chris Packham <judge.packham@gmail.com>,
	Konstantin Khomoutov <kostix+git@007spb.ru>,
	Git mailing list <git@vger.kernel.org>
Subject: Re: [PATCH v8 3/3] http-backend: respect CONTENT_LENGTH for receive-pack
Date: Wed, 25 Jul 2018 20:41:31 +0200	[thread overview]
Message-ID: <CAM0VKjkSMqPy=N3_0HUNxpCFwusrD_XE5j7kMsE4L-79g2t_VA@mail.gmail.com> (raw)
In-Reply-To: <20180725145100.GA1959@jessie.local>

On Wed, Jul 25, 2018 at 4:51 PM Max Kirillov <max@max630.net> wrote:
>
> On Wed, Jul 25, 2018 at 02:14:35PM +0200, SZEDER Gábor wrote:
> >> +    # sometimes there is fatal error buit the result is still 200

> >> +    if grep 'fatal:' act.err
> >> +    then
> >> +            return 1
> >> +    fi
> >
> > I just happened to stumble upon a failure because of 'fatal: the
> > remote end hung up unexpectedly' in the test 'push plain'.
>
> Did it happen once or repeated? It is rather strange, that
> one shoud not fail. Which OS it was?

Only once, so far.  It was one of my OSX build jobs on Travis CI, but
I don't know what OSX version is used.

'act.err' contained this (which will get line-wrapped, I'm afraid):

++handler_type=receive
++shift
++env CONTENT_TYPE=application/x-git-receive-pack-request
QUERY_STRING=/repo.git/git-receive-pack
'PATH_TRANSLATED=/Users/travis/t/trash
dir.t5562/.git/git-receive-pack' GIT_HTTP_EXPORT_ALL=TRUE
REQUEST_METHOD=POST
/Users/travis/build/szeder/git-cooking-topics-for-travis-ci/t/t5562/invoke-with-content-length.pl
push_body git http-backend
<...128 zero bytes...>fatal: the remote end hung up unexpectedly

I couldn't reproduce it on my Linux box.

> There have been doubds that a random incoming signal can
> trigger such a failure.
>
> > What does that "sometimes" in the above comment mean, and how often
> > does such a failure happen?  I see these patches are in 'pu' for over
> > a month now, so based on the number of reflog entries since then it
> > happened once from about 30-35 builds on Travis CI so far.
>
> "sometimes" here means "for some kinds of fatal error
> failure", there is nothing random in it.

> >> +    ! verify_http_result "200 OK"
> >
> > ... this function would return error (because of that 'if grep fatal:
> > ...' statement) without even looking at the status, but the test would
> > still succeed.  Is that really the desired behavior here?
>
> Yes, it is a desired behavior. A failure is expected here,
> and the failure does not show up as non-200 status, as
> described above.

OK, then I misunderstood that comment.

Perhaps a different wording could make it slightly better?  E.g. "In
some of these tests ..." instead of that "sometimes".  Dunno.