On Wed, May 19 2021, Jonathan Nieder wrote:
> Hi,
>
> (Danger, jrn is wading into error handling again...)
>
> At $DAYJOB we are setting up some alerting for some bot fleets and
> developer workstations, using trace2 as the data source. Having
> trace2 has been great --- combined with gradual weekly rollouts of
> "next", it helps us to understand quickly when a change is creating a
> regression for users, which hopefully improves the quality of Git for
> everyone.
>
> One kind of signal we haven't been able to make good use of is error
> rates. The problem is that a die() call can be an indication of
>
> a. the user asked to do something that isn't sensible, and we kindly
> rebuked the user
>
> b. we contacted a server, and the server was not happy with our
> request
>
> c. the local Git repository is corrupt
>
> d. we ran out of resources (e.g., disk space)
>
> e. we encountered an internal error in handling the user's
> legitimate request
> [...]
> Further down the line I can imagine making use of git_error_code
> elsewhere for e.g. some limited retries of the corresponding
> transaction when we fail to lock a file.
>
> Thoughts? Good idea? Bad idea?
Having read the thread at large (and some of this is a more general
response) a few points, not against or as a retort to this, just related
thoughts, complimentary suggestions etc:
1. As shown in my f6d25d78789 (api docs: document that BUG() emits a
trace2 error event, 2021-04-13) all of BUG/die/error/warning just
emit "error" under trace2.
It seems to me a good place to start with this effort would be for
someone to split that up. It requires changing the trace2 schema,
but it can be done in some backwards compatible way. Perhaps event:
error, error_type: [bug,die,error,warning] ?
1.5. Split up error_errno() from error() for trace2 purposes? This gets
you partway to your "d".
2. Similarly we need to log the correct line numbers for
die/error/warning. They need to be a macro/function like BUG() /
BUG_fl().
3. You can then key error events/frequencies on the "fmt".
4. To the extent tha #3 isn't true on client machines due to i18n we
could change the API in a backwards-compatible way from
e.g. error(_("string") to error(_N("string")). We'd then always
transmit the C locale "fmt".
Basically I wonder if a more granular approach with just better logging
of information we have now (but lose in trace2) + maybe some split-up of
the current functions, e.g. having a user_error() distinct from
repository_error() or whatever wouldn't get us most/all of the way to
this.
> Further down the line I can imagine making use of git_error_code
> elsewhere for e.g. some limited retries of the corresponding
> transaction when we fail to lock a file.
Maybe, but that seems highly problem-dependant, and not e.g. something
where we'd like to just do a blind retry in one of our own porcelain
tools if a plumbing one failed with a "had an issue, retries might work"
code.
