Implement a linter to discourage future usage of Unavailable code

The prior commit fixes inelligible usages of Unavailable status codes.
To prevent this situation from happening in the future, this commit
implements a linter that it warns occurrences where Unavailable code is
used. Engineers can ignore it by adding a comment, but at least this
practice catches their eyes.

8 files changed, 140 insertions, 3 deletions

diff --git a/.golangci.yml b/.golangci.yml
index c41e1df5d..aaca0828f 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -144,6 +144,9 @@ linters-settings:
           included-functions:
             - fmt.Errorf
             - gitlab.com/gitlab-org/gitaly/v16/internal/structerr.*
+        unavailable_code:
+          included-functions:
+            - gitlab.com/gitlab-org/gitaly/v16/internal/structerr.NewUnavailable
 
 issues:
   exclude-use-default: false
diff --git a/STYLE.md b/STYLE.md
index 81132e77c..bbd390894 100644
--- a/STYLE.md
+++ b/STYLE.md
@@ -163,6 +163,30 @@ message FrobnicateError {
 }
 ```
 
+### Unavailable code
+
+The Unavailable status code is reserved for cases that clients are encouraged to retry. The most suitable use cases for
+this status code are in interceptors or network-related components such as load-balancing. The official documentation
+differentiates the usage of status codes as the following:
+
+> (a) Use UNAVAILABLE if the client can retry just the failing call.
+> (b) Use ABORTED if the client should retry at a higher level
+> (c) Use FAILED_PRECONDITION if the client should not retry until the
+> system state has been explicitly fixed
+
+In the past we've had multiple places in the source code where an error from sending a streaming message was captured
+and wrapped in an `Unavailable` code. This status code is often not correct because it can raise other less common
+errors, such as buffer overflow (`ResourceExhausted`), max message size exceeded (`ResourceExhausted`), or encoding
+failure (`Internal`). It's more appropriate for the handler to propagate the error up as an `Aborted` status code.
+
+Another common misused pattern is wrapping the spawned process exit code. In many cases, if Gitaly can intercept the
+exit code or/and error from stderr, it must have a precise error code (`InvalidArgument`, `NotFound`, `Internal`).
+However, Git processes may exit with 128 status code and un-parseable stderr. We can intercept it as an operation was
+rejected because the system is not in a state where it can be executed (resource inaccessible, invalid refs, etc.).
+For these situations, `FailedPrecondition` is the most suitable choice.
+
+Thus, gRPC handlers should avoid using `Unavailable` status code.
+
 ## Logging
 
 ### Use context-based logging
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index cebbccb09..f12e2818f 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -777,7 +777,8 @@ remote: error resource exhausted, please try again later
 		},
 		{
 			name: "other error - status code is hidden",
-			err:  structerr.NewUnavailable("server is not available"),
+			//nolint:gitaly-linters
+			err: structerr.NewUnavailable("server is not available"),
 			expectedStderr: `
 remote: error executing git hook
 `,
diff --git a/internal/structerr/error.go b/internal/structerr/error.go
index 541bec49e..184b0da90 100644
--- a/internal/structerr/error.go
+++ b/internal/structerr/error.go
@@ -177,8 +177,22 @@ func NewResourceExhausted(format string, a ...any) Error {
 	return newError(codes.ResourceExhausted, format, a...)
 }
 
-// NewUnavailable constructs a new error code with the Unavailable error code. Please refer to New
-// for further details.
+// NewUnavailable constructs a new error code with the Unavailable error code. Please refer to New for further details.
+// Please note that the Unavailable status code is a signal telling clients to retry automatically. This auto-retry
+// mechanism is handled at the library layer, without client consensus. Typically, it is used for the situations where
+// the gRPC is not available or is not responding. Here are some discrete examples:
+//
+//   - Server downtime: The server hosting the gRPC service is down for maintenance or has crashed.
+//   - Network issues: Connectivity problems between the client and server, like network congestion or a broken connection,
+//     can cause the service to appear unavailable.
+//   - Load balancing failure: In a distributed system, the load balancer may be unable to route the client's request to a
+//     healthy instance of the gRPC service. This can happen if all instances are down or if the load balancer is
+//     misconfigured.
+//   - TLS/SSL handshake failure: If there's a problem during the TLS/SSL handshake between the client and the server, the
+//     connection may fail, leading to an UNAVAILABLE status code.
+//
+// Thus, this status code should be used by interceptors or network-related components. gRPC handlers should use another
+// status code instead.
 func NewUnavailable(format string, a ...any) Error {
 	return newError(codes.Unavailable, format, a...)
 }
diff --git a/tools/golangci-lint/gitaly/lint.go b/tools/golangci-lint/gitaly/lint.go
index 1d76a9400..6840b15ac 100644
--- a/tools/golangci-lint/gitaly/lint.go
+++ b/tools/golangci-lint/gitaly/lint.go
@@ -23,6 +23,10 @@ func (p *analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
 				"included-functions",
 			),
 		}),
+		newUnavailableCodeAnalyzer(&unavailableCodeAnalyzerSettings{IncludedFunctions: p.configStringSlicesAt(
+			unavailableCodeAnalyzerName,
+			"included-functions",
+		)}),
 	}
 }
 
diff --git a/tools/golangci-lint/gitaly/testdata/src/unavailable_code/unavailable_code_test.go b/tools/golangci-lint/gitaly/testdata/src/unavailable_code/unavailable_code_test.go
new file mode 100644
index 000000000..5493e3f7a
--- /dev/null
+++ b/tools/golangci-lint/gitaly/testdata/src/unavailable_code/unavailable_code_test.go
@@ -0,0 +1,21 @@
+package unavailable_code
+
+import (
+	"fmt"
+)
+
+func NewUnavailable(msg string) error {
+	return fmt.Errorf("unavailable: %s", msg)
+}
+
+func NewAborted(msg string) error {
+	return fmt.Errorf("aborted: %s", msg)
+}
+
+func errorWrapOkay() {
+	_ = NewAborted("hello world")
+}
+
+func errorWrapNotOkay() {
+	_ = NewUnavailable("hello world") // please avoid using the Unavailable status code: https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md?plain=0#unavailable-code
+}
diff --git a/tools/golangci-lint/gitaly/unavailable_code.go b/tools/golangci-lint/gitaly/unavailable_code.go
new file mode 100644
index 000000000..9546b1871
--- /dev/null
+++ b/tools/golangci-lint/gitaly/unavailable_code.go
@@ -0,0 +1,48 @@
+package main
+
+import (
+	"go/ast"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+const unavailableCodeAnalyzerName = "unavailable_code"
+
+type unavailableCodeAnalyzerSettings struct {
+	IncludedFunctions []string `mapstructure:"included-functions"`
+}
+
+// newErrorWrapAnalyzer warns if Unavailable status code is used. Unavailable status code is reserved to signal server's
+// unavailability. It should be used by some specific components. gRPC handlers should typically avoid this type of
+// error.
+// For more information:
+// https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md?plain=0#unavailable-code
+func newUnavailableCodeAnalyzer(settings *unavailableCodeAnalyzerSettings) *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name: unavailableCodeAnalyzerName,
+		Doc:  `discourage the usage of Unavailable status code`,
+		Run:  runUnavailableCodeAnalyzer(settings.IncludedFunctions),
+	}
+}
+
+func runUnavailableCodeAnalyzer(rules []string) func(*analysis.Pass) (interface{}, error) {
+	return func(pass *analysis.Pass) (interface{}, error) {
+		matcher := NewMatcher(pass)
+		for _, file := range pass.Files {
+			ast.Inspect(file, func(n ast.Node) bool {
+				if call, ok := n.(*ast.CallExpr); ok {
+					if matcher.MatchFunction(call, rules) {
+						pass.Report(analysis.Diagnostic{
+							Pos:            call.Pos(),
+							End:            call.End(),
+							Message:        "please avoid using the Unavailable status code: https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md?plain=0#unavailable-code",
+							SuggestedFixes: nil,
+						})
+					}
+				}
+				return true
+			})
+		}
+		return nil, nil
+	}
+}
diff --git a/tools/golangci-lint/gitaly/unavailable_code_test.go b/tools/golangci-lint/gitaly/unavailable_code_test.go
new file mode 100644
index 000000000..39d97e788
--- /dev/null
+++ b/tools/golangci-lint/gitaly/unavailable_code_test.go
@@ -0,0 +1,22 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestUnavailableCode(t *testing.T) {
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get wd: %s", err)
+	}
+
+	testdata := filepath.Join(wd, "testdata")
+	analyzer := newUnavailableCodeAnalyzer(&unavailableCodeAnalyzerSettings{IncludedFunctions: []string{
+		"unavailable_code.NewUnavailable",
+	}})
+	analysistest.Run(t, testdata, analyzer, "unavailable_code")
+}