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

[Jeff King <peff@peff.net>]

On Sat, Jul 25, 2020 at 04:01:25PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think it was this one:
> >
> >   https://lore.kernel.org/git/20160927191955.mympqgylrxhkp24n@sigill.intra.peff.net/
> >
> > I still think it's a good idea, but the hard part is lib-ifying all of
> > the functions that call die() to instead return an error up the stack
> > (and handle cleanup, etc). Which is why I never really pushed it
> > further. But if we're going to be lib-ifying such calls anyway, I think
> > it's nice to do this flexible thing (from their perspective it's no more
> > work to trigger the callback than it is to call error() and return).
> 
> Yeah, I almost 80%+ agree.
> 
> The remainder of 20% is that I am not so convinced that (fmt_string
> + va_list) that isn't pretty much usable for anything but generating
> human readable error messages is enough.  It is certainly a good
> enough replacement for (and much better than) the approach to
> prepare an error string in a "struct strbuf err" that was passeed by
> the caller, but I am not sure if that is a good bar to aim to pass
> to begin with ;-).

Yeah, I certainly agree that just passing around strings does not
provide as much information as some callers would like. But I think
introducing a system of machine-readable error tokens would be
cumbersome for a lot of cases, to the point that it may tip this from
"seems like an easy enough change" to something that nobody wants to
take on.

One in-between step that would be pretty easy is to record errno when
available (i.e., if error_errno() becomes error_ctx_errno(), then we
just stuff that errno value into the error_context struct). That would
let us reliably propagate errno up the stack, or even give intermediate
callers the opportunity to munge it (e.g., if a ref lookup failed with
EISDIR while opening a file, it might convert that to ENOENT as it
propagates up the stack).

-Peff