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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jul 25, 2018, 18:41 UTC
Message-ID: <CAM0VKjkSMqPy=N3_0HUNxpCFwusrD_XE5j7kMsE4L-79g2t_VA@mail.gmail.com>
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

Show 10 quoted lines

> >> +    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.

Show 10 quoted lines

> 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.

Show 9 quoted lines

> >> +    ! 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.