RFC: error codes on exit

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

and these different cases do not all motivate the same response. (E.g., if (c) affects just a single bot but produces a high error rate from that bot, we shouldn't be alarmed; if (d) is happening on a bot, then we should look into giving it more disk; if (e) is increasing significantly during a rollout then we should roll back quickly.)

In order to do this, I would like to annotate "exit" events with a classification of the error. I'm not too opinionated about what that classification looks like (bikeshedding welcome!) --- e.g., something like the enumeration at https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto is likely to work fine.

(I'm particularly fond of how that maps to HTTP statuses. See also https://github.com/abseil/abseil-cpp/blob/HEAD/absl/status/status.h for an example of using that kind of enumeration within a single process.)

The API could look something like

	--- a/cache.h
	+++ b/cache.h
	@@ -590,6 +590,15 @@ int is_git_directory(const char *path);
	  */
	 int is_nonbare_repository_dir(struct strbuf *path);

	+enum git_error_code {
	+	/*
	+	 * Not an error (= HTTP 200)
	+	 */
	+	OK = 0,
	+};
	+NORETURN void fatal(enum git_error_code code, const char *err, ...)
	+	__attribute__((format (printf, 2, 3)));
	+
	 #define READ_GITFILE_ERR_STAT_FAILED 1
	 #define READ_GITFILE_ERR_NOT_A_FILE 2
	 #define READ_GITFILE_ERR_OPEN_FAILED 3

(with new error codes added when they first get used) and a typical caller could look like

	Subject: xsize_t: tag "cannot handle files this big" as a failed precondition

	Unlike retriable errors, failed preconditions indicate that some
	aspect of the state needs to be changed in order to recover.  Mark
	this error as such to make signals from monitoring in controlled
	environments (e.g., bot fleets or corporate installations of Git)
	easier to understand.

	Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
[...]
	+       /*
	+        * The system is not in a state required for the operation to succeed.
	+        * For example, a file on disk is larger than we can handle.
	+        * (= HTTP 400)
	+        */
	+       FAILED_PRECONDITION = 9,
[...]
	 static inline size_t xsize_t(off_t len)
	 {
		if (len < 0 || len > SIZE_MAX)
	-               die("Cannot handle files this big");
	+               fatal(FAILED_PRECONDITION, "Cannot handle files this big");

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?

Thanks, Jonathan