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

From: Jonathan Nieder <[email-masked]>
Date: Wed, 3 Dec 2014 19:01:33 -0800

Signed-off-by: Jonathan Nieder <[email-masked]>
---
Junio C Hamano wrote:
> Jonathan Nieder <[email-masked]> 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);