Re: [RFC PATCH] Modify fetch-pack to no longer die on error?

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 24, 2020, 23:07 UTC
Message-ID: <xmqqblk4jyvc.fsf@gitster.c.googlers.com>
In-Reply-To: <20200724223844.2723397-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 13 quoted lines

> We've had a few instances where a lazy fetch in a partial clone fails,
> leading to a fatal error, when the calling code could have easily
> recovered - in other words, the severity of the bug should have just a
> wasted fetch instead of stopping the whole command. Part of the issue
> (and possibly the whole issue - I haven't looked at this beyond
> fetch-pack yet) is that fetch-pack dies whenever it encounters an
> error, so I took a look at fixing that.
>
> (Note that fetch-pack is sometimes run through a remote helper, meaning
> that we could leave the die() invocations in and just make sure that we
> handle failure in the separate process correctly. But when the promisor
> remote is HTTP protocol v2 or SSH protocol v0/v2, this is not true -
> fetch_pack() is run in-process.)

That is easily fixable ;-) After all, we are talking about network latency once we "lazily fetch" during a main operation, so why not take advantage of the isolation that the operating system gives us?

> I think the best way for easy authorship and review is to convert each
> possibly-dying function into a function that either returns a
> possibly-null error string or returns success/failure and writes the
> error string into an "out" parameter.

Ugly.