comparison mercurial/bundle2.py @ 32024:ad41739c6b2b

bundle2: ignore errors seeking a bundle after an exception (issue4784)

Many have seen a "stream ended unexpectedly" error. This message is
raised from changegroup.readexactly() when a read(n) operation fails
to return exactly N bytes.

I believe most occurrences of this error in the wild stem from
the code changed in this patch. Before, if bundle2's part applicator
raised an Exception when processing/applying parts, the exception
handler would attempt to iterate the remaining parts. If I/O
during this iteration failed, it would likely raise the
"stream ended unexpectedly" exception.

The problem with this approach is that if we already encountered
an I/O error iterating the bundle2 data during application, then
any further I/O would almost certainly fail. If the original stream
were closed, changegroup.readexactly() would obtain an empty
string, triggering "stream ended unexpectedly" with "got 0." This is
the error message that users would see. What's worse is that the
original I/O related exception would be lost since a new exception
would be raised. This made debugging the actual I/O failure
effectively impossible.

This patch changes the exception handler for bundle2 application to
ignore errors when seeking the underlying stream. When the underlying
error is I/O related, the seek should fail fast and the original
exception will be re-raised. The output changes in
test-http-bad-server.t demonstrate this.

When the underlying error is not I/O related and the stream can be
seeked, the old behavior is preserved.

32023:a29580905771

    nbpart = 0
    try:
        for nbpart, part in iterparts:
            _processpart(op, part)
    except Exception as exc:
        for nbpart, part in iterparts:
            # consume the bundle content
            part.seek(0, 2)
        # Small hack to let caller code distinguish exceptions from bundle2
        # processing from processing the old format. This is mostly
        # needed to handle different return codes to unbundle according to the
        # type of bundle. We should probably clean up or drop this return code
        # craziness in a future version.

        if op.reply is not None:
            salvaged = op.reply.salvageoutput()
            replycaps = op.reply.capabilities
        exc._replycaps = replycaps
        exc._bundle2salvagedoutput = salvaged
        raise
    finally:
        repo.ui.debug('bundle2-input-bundle: %i parts total\n' % nbpart)

    return op

32024:ad41739c6b2b

    nbpart = 0
    try:
        for nbpart, part in iterparts:
            _processpart(op, part)
    except Exception as exc:
        # Any exceptions seeking to the end of the bundle at this point are
        # almost certainly related to the underlying stream being bad.
        # And, chances are that the exception we're handling is related to
        # getting in that bad state. So, we swallow the seeking error and
        # re-raise the original error.
        seekerror = False
        try:
            for nbpart, part in iterparts:
                # consume the bundle content
                part.seek(0, 2)
        except Exception:
            seekerror = True

        # Small hack to let caller code distinguish exceptions from bundle2
        # processing from processing the old format. This is mostly
        # needed to handle different return codes to unbundle according to the
        # type of bundle. We should probably clean up or drop this return code
        # craziness in a future version.

        if op.reply is not None:
            salvaged = op.reply.salvageoutput()
            replycaps = op.reply.capabilities
        exc._replycaps = replycaps
        exc._bundle2salvagedoutput = salvaged

        # Re-raising from a variable loses the original stack. So only use
        # that form if we need to.
        if seekerror:
            raise exc
        else:
            raise
    finally:
        repo.ui.debug('bundle2-input-bundle: %i parts total\n' % nbpart)

    return op