Re: [PATCH/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf)

> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> -extern int copy_fd(int ifd, int ofd);
>>> +extern int copy_fd(int ifd, int ofd, struct strbuf *err);
>>
>> It is not limited to this single function, but what contract do we
>> envision this "error messages are given back to the caller via
>> strbuf" convention should give between the callers and the callee?
>
> Here's a draft for documentation on that.

>  Documentation/technical/api-error-handling.txt | 75 ++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/technical/api-error-handling.txt
>
> diff --git a/Documentation/technical/api-error-handling.txt
> b/Documentation/technical/api-error-handling.txt
> new file mode 100644
> index 0000000..fc68db1
> --- /dev/null
> +++ b/Documentation/technical/api-error-handling.txt
> @@ -0,0 +1,75 @@
> +Error reporting in git
> +======================
> +
> +`die`, `usage`, `error`, and `warning` report errors of various
> +kinds.
> +
> +- `die` is for fatal application errors.  It prints a message to
> +  the user and exits with status 128.
> +
> +- `usage` is for errors in command line usage.  After printing its
> +  message, it exits with status 129.  (See also `usage_with_options`
> +  in the link:api-parse-options.html[parse-options API].)
> +
> +- `error` is for non-fatal library errors.  It prints a message
> +  to the user and returns -1 for convenience in signaling the error
> +  to the caller.
> +
> +- `warning` is for reporting situations that probably should not
> +  occur but which the user (and Git) can continue to work around
> +  without running into too many problems.  Like `error`, it
> +  returns -1 after reporting the situation to the caller.
> +
> +Customizable error handlers
> +---------------------------
> +
> +The default behavior of `die` and `error` is to write a message to
> +stderr and then exit or return as appropriate.  This behavior can be
> +overridden using `set_die_routine` and `set_error_routine`.  For
> +example, "git daemon" uses set_die_routine to write the reason `die`
> +was called to syslog before exiting.
> +
> +Library errors
> +--------------
> +
> +Functions return a negative integer on error.  Details beyond that
> +vary from function to function:
> +
> +- Some functions return -1 for all errors.  Others return a more
> +  specific value depending on how the caller might want to react
> +  to the error.
> +
> +- Some functions report the error to stderr with `error`,
> +  while others leave that for the caller to do.
> +
> +- errno is not meaningful on return from most functions (except
> +  for thin wrappers for system calls).
> +
> +Check the function's API documentation to be sure.
> +
> +Caller-handled errors
> +---------------------
> +
> +An increasing number of functions take a parameter 'struct strbuf *err'.
> +On error, such functions append a message about what went wrong to the
> +'err' strbuf.  The message is meant to be complete enough to be passed
> +to `die` or `error` as-is.  For example:
> +
> +	if (ref_transaction_commit(transaction, &err))
> +		die("%s", err.buf);
> +
> +The 'err' parameter will be untouched if no error occured, so multiple
> +function calls can be chained:
> +
> +	t = ref_transaction_begin(&err);
> +	if (!t ||
> +	    ref_transaction_update(t, "HEAD", ..., &err) ||
> +	    ret_transaction_commit(t, &err))
> +		die("%s", err.buf);
> +
> +The 'err' parameter must be a pointer to a valid strbuf.  To silence
> +a message, pass a strbuf that is explicitly ignored:
> +
> +	if (thing_that_can_fail_in_an_ignorable_way(..., &err))
> +		/* This failure is okay. */
> +		strbuf_reset(&err);