From 88433a9f2cf528705d1ec89d0248ab6edba76a82 Mon Sep 17 00:00:00 2001
From: Josh Deprez <jd@buildkite.com>
Date: Thu, 3 Oct 2024 18:10:29 +1000
Subject: [PATCH] Exit 94 if a mirror lock times out

---
 internal/job/checkout.go    | 44 ++++++++++++++++++++++++++++---------
 internal/job/executor.go    |  4 ++--
 internal/job/shell/shell.go | 10 ++++-----
 3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/internal/job/checkout.go b/internal/job/checkout.go
index e5caa25f80..4246cc731e 100644
--- a/internal/job/checkout.go
+++ b/internal/job/checkout.go
@@ -160,23 +160,33 @@ func (e *Executor) CheckoutPhase(ctx context.Context) error {
 			roko.WithMaxAttempts(3),
 			roko.WithStrategy(roko.Constant(2*time.Second)),
 		).DoWithContext(ctx, func(r *roko.Retrier) error {
-			err := e.defaultCheckoutPhase(ctx)
-			if err == nil {
+
+			var errLockTimeout ErrTimedOutAcquiringLock
+
+			switch err := e.defaultCheckoutPhase(ctx); {
+			case err == nil:
 				return nil
-			}
 
-			switch {
 			case shell.IsExitError(err) && shell.GetExitCode(err) == -1:
 				e.shell.Warningf("Checkout was interrupted by a signal")
 				r.Break()
+				return err
+
+			case errors.As(err, &errLockTimeout):
+				e.shell.Warningf("Checkout could not acquire the %s lock before timing out", errLockTimeout.Name)
+				r.Break()
+				// 94 chosen by fair die roll
+				return &shell.ExitError{Code: 94, Err: err}
 
 			case errors.Is(err, context.Canceled):
 				e.shell.Warningf("Checkout was cancelled")
 				r.Break()
+				return err
 
 			case errors.Is(ctx.Err(), context.Canceled):
 				e.shell.Warningf("Checkout was cancelled due to context cancellation")
 				r.Break()
+				return err
 
 			default:
 				e.shell.Warningf("Checkout failed! %s (%s)", err, r)
@@ -206,12 +216,9 @@ func (e *Executor) CheckoutPhase(ctx context.Context) error {
 				// Now make sure the build directory exists again before we try
 				// to checkout again, or proceed and run hooks which presume the
 				// checkout dir exists
-				if err := e.createCheckoutDir(); err != nil {
-					return err
-				}
+				return e.createCheckoutDir()
 			}
 
-			return err
 		}); err != nil {
 			return err
 		}
@@ -304,7 +311,10 @@ func (e *Executor) updateGitMirror(ctx context.Context, repository string) (stri
 	defer canc()
 	mirrorCloneLock, err := e.shell.LockFile(cloneCtx, mirrorDir+".clonelock")
 	if err != nil {
-		return "", err
+		if errors.Is(err, context.DeadlineExceeded) {
+			return "", ErrTimedOutAcquiringLock{Name: "clone", Err: err}
+		}
+		return "", fmt.Errorf("unable to acquire clone lock: %w", err)
 	}
 	defer mirrorCloneLock.Unlock()
 
@@ -343,7 +353,10 @@ func (e *Executor) updateGitMirror(ctx context.Context, repository string) (stri
 	defer canc()
 	mirrorUpdateLock, err := e.shell.LockFile(updateCtx, mirrorDir+".updatelock")
 	if err != nil {
-		return "", err
+		if errors.Is(err, context.DeadlineExceeded) {
+			return "", ErrTimedOutAcquiringLock{Name: "update", Err: err}
+		}
+		return "", fmt.Errorf("unable to acquire update lock: %w", err)
 	}
 	defer mirrorUpdateLock.Unlock()
 
@@ -407,6 +420,17 @@ func (e *Executor) updateGitMirror(ctx context.Context, repository string) (stri
 	return mirrorDir, nil
 }
 
+type ErrTimedOutAcquiringLock struct {
+	Name string
+	Err  error
+}
+
+func (e ErrTimedOutAcquiringLock) Error() string {
+	return fmt.Sprintf("timed out acquiring %s lock: %v", e.Name, e.Err)
+}
+
+func (e ErrTimedOutAcquiringLock) Unwrap() error { return e.Err }
+
 // updateRemoteURL updates the URL for 'origin'. If gitDir == "", it assumes the
 // local repo is in the current directory, otherwise it includes --git-dir.
 // If the remote has changed, it logs some extra information. updateRemoteURL
diff --git a/internal/job/executor.go b/internal/job/executor.go
index 280bb02abd..090339e092 100644
--- a/internal/job/executor.go
+++ b/internal/job/executor.go
@@ -515,8 +515,8 @@ func (e *Executor) runWrappedShellScriptHook(ctx context.Context, hookName strin
 		// so it may inform the Buildkite API
 		if shell.IsExitError(err) {
 			return &shell.ExitError{
-				Code:    exitCode,
-				Message: fmt.Sprintf("The %s hook exited with status %d", hookName, exitCode),
+				Code: exitCode,
+				Err:  fmt.Errorf("The %s hook exited with status %d", hookName, exitCode),
 			}
 		}
 
diff --git a/internal/job/shell/shell.go b/internal/job/shell/shell.go
index 040a7b617c..cde8c198ad 100644
--- a/internal/job/shell/shell.go
+++ b/internal/job/shell/shell.go
@@ -657,11 +657,11 @@ func IsExitError(err error) bool {
 
 // ExitError is an error that carries a shell exit code
 type ExitError struct {
-	Code    int
-	Message string
+	Code int
+	Err  error
 }
 
 // Error returns the string message and fulfils the error interface
-func (ee *ExitError) Error() string {
-	return ee.Message
-}
+func (ee *ExitError) Error() string { return ee.Err.Error() }
+
+func (ee *ExitError) Unwrap() error { return ee.Err }