From 28f4c3ddbb70ed7992b6694c7c9a56f16f5680b2 Mon Sep 17 00:00:00 2001
From: Fedor Korotkov <fedor.korotkov@gmail.com>
Date: Thu, 25 Apr 2024 19:21:39 +0200
Subject: [PATCH] Don't hide cancellation reason (#363)

* Don't hide cancellation reason

Otherwise, we assume a task has timeout when in reality it could've been externally interrupted.

* revert log

* Less changes

* Removed whitespace
---
 internal/executor/executor.go           | 5 +++--
 internal/executor/shell.go              | 8 ++++----
 internal/executor/shell_subunix_test.go | 2 +-
 internal/executor/shell_unix_test.go    | 4 ++--
 internal/executor/shell_windows_test.go | 2 +-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/internal/executor/executor.go b/internal/executor/executor.go
index d94d6f80..44e3051c 100644
--- a/internal/executor/executor.go
+++ b/internal/executor/executor.go
@@ -52,6 +52,7 @@ type StepResult struct {
 
 var (
 	ErrStepExit = errors.New("executor step requested to terminate execution")
+	ErrTimedOut = errors.New("timed out")
 )
 
 func NewExecutor(
@@ -215,7 +216,7 @@ func (executor *Executor) RunBuild(ctx context.Context) {
 	// Normal timeout-bounded context
 	timeout := time.Duration(response.TimeoutInSeconds) * time.Second
 
-	timeoutCtx, timeoutCtxCancel := context.WithTimeout(ctx, timeout)
+	timeoutCtx, timeoutCtxCancel := context.WithTimeoutCause(ctx, timeout, ErrTimedOut)
 	defer timeoutCtxCancel()
 
 	// Like timeout-bounded context, but extended by 5 minutes
@@ -500,7 +501,7 @@ func (executor *Executor) performStep(ctx context.Context, currentStep *api.Comm
 				signaledToExit = ws.Signaled()
 			}
 		}
-		if err == TimeOutError {
+		if errors.Is(err, ErrTimedOut) {
 			signaledToExit = false
 		}
 	case *api.Command_BackgroundScriptInstruction:
diff --git a/internal/executor/shell.go b/internal/executor/shell.go
index 45450038..daee9a3d 100644
--- a/internal/executor/shell.go
+++ b/internal/executor/shell.go
@@ -12,6 +12,7 @@ import (
 	"os"
 	"os/exec"
 	"os/signal"
+	"strings"
 	"syscall"
 	"time"
 )
@@ -23,8 +24,6 @@ type ShellOutputWriter struct {
 	handler ShellOutputHandler
 }
 
-var TimeOutError = errors.New("timed out")
-
 func (writer ShellOutputWriter) Write(bytes []byte) (int, error) {
 	return writer.handler(bytes)
 }
@@ -61,7 +60,8 @@ func ShellCommandsAndWait(
 
 	select {
 	case <-ctx.Done():
-		handler([]byte("\nTimed out!"))
+		errorMessage := fmt.Sprintf("%v!", context.Cause(ctx))
+		handler([]byte("\n" + strings.ToUpper(errorMessage[:1]) + errorMessage[1:]))
 
 		processdumper.Dump()
 
@@ -69,7 +69,7 @@ func ShellCommandsAndWait(
 			handler([]byte(fmt.Sprintf("\nFailed to kill a timed out shell session: %s", err)))
 		}
 
-		return cmd, TimeOutError
+		return cmd, context.Cause(ctx)
 	case <-done:
 		var forcePiperClosure bool
 
diff --git a/internal/executor/shell_subunix_test.go b/internal/executor/shell_subunix_test.go
index 5fa17824..1c915a86 100644
--- a/internal/executor/shell_subunix_test.go
+++ b/internal/executor/shell_subunix_test.go
@@ -16,7 +16,7 @@ import (
 // the shell spawned in ShellCommandsAndGetOutput() has been placed into, thus killing
 // it's children processes.
 func TestProcessGroupTermination(t *testing.T) {
-	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
+	ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, ErrTimedOut)
 	defer cancel()
 
 	success, output := ShellCommandsAndGetOutput(ctx, []string{"sleep 86400 & echo target PID is $! ; sleep 60"}, nil)
diff --git a/internal/executor/shell_unix_test.go b/internal/executor/shell_unix_test.go
index 52f5dd63..cec3f38f 100644
--- a/internal/executor/shell_unix_test.go
+++ b/internal/executor/shell_unix_test.go
@@ -112,7 +112,7 @@ func Test_ShellCommands_CustomWorkingDir_Unix(t *testing.T) {
 }
 
 func Test_ShellCommands_Timeout_Unix(t *testing.T) {
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrTimedOut)
 	defer cancel()
 
 	_, output := ShellCommandsAndGetOutput(ctx, []string{"sleep 60"}, nil)
@@ -124,7 +124,7 @@ func Test_ShellCommands_Timeout_Unix(t *testing.T) {
 }
 
 func TestChildrenProcessesAreCancelled(t *testing.T) {
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
+	ctx, cancel := context.WithTimeoutCause(context.Background(), time.Second*5, ErrTimedOut)
 	defer cancel()
 
 	success, output := ShellCommandsAndGetOutput(ctx, []string{"sleep 60 & sleep 10"}, nil)
diff --git a/internal/executor/shell_windows_test.go b/internal/executor/shell_windows_test.go
index 6fa87966..525ba7b5 100644
--- a/internal/executor/shell_windows_test.go
+++ b/internal/executor/shell_windows_test.go
@@ -53,7 +53,7 @@ func TestMain(m *testing.M) {
 // TestProcessGroupTermination ensures that we terminate all processes we've automatically
 // tainted by assigning a job object to a shell spawned in ShellCommandsAndGetOutput().
 func TestJobObjectTermination(t *testing.T) {
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, ErrTimedOut)
 	defer cancel()
 
 	success, output := ShellCommandsAndGetOutput(ctx, []string{os.Args[0]},