diff --git a/act/container/container_types.go b/act/container/container_types.go index a1b2f01c..16b9ed99 100644 --- a/act/container/container_types.go +++ b/act/container/container_types.go @@ -6,6 +6,7 @@ package container import ( "context" + "fmt" "io" "gitea.com/gitea/runner/act/common" @@ -13,6 +14,13 @@ import ( "github.com/docker/go-connections/nat" ) +// ExitCodeError reports a non-zero process exit code from a container command. +type ExitCodeError int + +func (e ExitCodeError) Error() string { + return fmt.Sprintf("Process completed with exit code %d.", int(e)) +} + // NewContainerInput the input for the New function type NewContainerInput struct { Image string diff --git a/act/container/docker_run.go b/act/container/docker_run.go index bb9d8fb6..3aa891dc 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -633,14 +633,10 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo return fmt.Errorf("failed to inspect exec: %w", err) } - switch inspectResp.ExitCode { - case 0: + if inspectResp.ExitCode == 0 { return nil - case 127: - return fmt.Errorf("exitcode '%d': command not found, please refer to https://github.com/nektos/act/issues/107 for more information", inspectResp.ExitCode) - default: - return fmt.Errorf("exitcode '%d': failure", inspectResp.ExitCode) } + return ExitCodeError(inspectResp.ExitCode) } } @@ -930,7 +926,7 @@ func (cr *containerReference) wait() common.Executor { return nil } - return fmt.Errorf("exit with `FAILURE`: %v", statusCode) + return ExitCodeError(statusCode) } } diff --git a/act/container/docker_run_test.go b/act/container/docker_run_test.go index beae70ca..9194d8c4 100644 --- a/act/container/docker_run_test.go +++ b/act/container/docker_run_test.go @@ -23,6 +23,7 @@ import ( "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) func TestDocker(t *testing.T) { @@ -85,6 +86,11 @@ func (m *mockDockerClient) ContainerExecInspect(ctx context.Context, execID stri return args.Get(0).(types.ContainerExecInspect), args.Error(1) } +func (m *mockDockerClient) ContainerWait(ctx context.Context, containerID string, condition container.WaitCondition) (<-chan container.WaitResponse, <-chan error) { + args := m.Called(ctx, containerID, condition) + return args.Get(0).(<-chan container.WaitResponse), args.Get(1).(<-chan error) +} + func (m *mockDockerClient) CopyToContainer(ctx context.Context, id, path string, content io.Reader, options types.CopyToContainerOptions) error { args := m.Called(ctx, id, path, content, options) return args.Error(0) @@ -174,12 +180,43 @@ func TestDockerExecFailure(t *testing.T) { } err := cr.exec([]string{""}, map[string]string{}, "user", "workdir")(ctx) - assert.Error(t, err, "exit with `FAILURE`: 1") //nolint:testifylint // pre-existing issue from nektos/act + var exitErr ExitCodeError + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, ExitCodeError(1), exitErr) + assert.Equal(t, "Process completed with exit code 1.", err.Error()) conn.AssertExpectations(t) client.AssertExpectations(t) } +func TestDockerWaitFailure(t *testing.T) { + ctx := context.Background() + + statusCh := make(chan container.WaitResponse, 1) + statusCh <- container.WaitResponse{StatusCode: 2} + errCh := make(chan error, 1) + + client := &mockDockerClient{} + client.On("ContainerWait", ctx, "123", container.WaitConditionNotRunning). + Return((<-chan container.WaitResponse)(statusCh), (<-chan error)(errCh)) + + cr := &containerReference{ + id: "123", + cli: client, + input: &NewContainerInput{ + Image: "image", + }, + } + + err := cr.wait()(ctx) + var exitErr ExitCodeError + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, ExitCodeError(2), exitErr) + assert.Equal(t, "Process completed with exit code 2.", err.Error()) + + client.AssertExpectations(t) +} + func TestDockerCopyTarStream(t *testing.T) { ctx := context.Background() diff --git a/act/container/host_environment.go b/act/container/host_environment.go index 68414187..121266b8 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -372,6 +372,10 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st } err = cmd.Wait() if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return ExitCodeError(exitErr.ExitCode()) + } return err } if tty != nil { diff --git a/act/container/host_environment_test.go b/act/container/host_environment_test.go index a99a418b..4101e8ba 100644 --- a/act/container/host_environment_test.go +++ b/act/container/host_environment_test.go @@ -11,6 +11,7 @@ import ( "os" "path" "path/filepath" + "runtime" "testing" "gitea.com/gitea/runner/act/common" @@ -74,6 +75,31 @@ func TestGetContainerArchive(t *testing.T) { assert.ErrorIs(t, err, io.EOF) } +func TestHostEnvironmentExecExitCode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses POSIX shell") + } + dir := t.TempDir() + ctx := context.Background() + e := &HostEnvironment{ + Path: filepath.Join(dir, "path"), + TmpDir: filepath.Join(dir, "tmp"), + ToolCache: filepath.Join(dir, "tool_cache"), + ActPath: filepath.Join(dir, "act_path"), + StdOut: io.Discard, + Workdir: filepath.Join(dir, "path"), + } + for _, p := range []string{e.Path, e.TmpDir, e.ToolCache, e.ActPath} { + assert.NoError(t, os.MkdirAll(p, 0o700)) //nolint:testifylint // test setup + } + + err := e.Exec([]string{"sh", "-c", "exit 3"}, map[string]string{"PATH": os.Getenv("PATH")}, "", "")(ctx) + var exitErr ExitCodeError + require.ErrorAs(t, err, &exitErr) + assert.Equal(t, ExitCodeError(3), exitErr) + assert.Equal(t, "Process completed with exit code 3.", err.Error()) +} + func TestHostEnvironmentRemoveCleansWorkdir(t *testing.T) { logger := logrus.New() ctx := common.WithLogger(context.Background(), logrus.NewEntry(logger)) diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index 2001dea4..ed9f7062 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -24,6 +24,13 @@ type jobInfo interface { result(result string) } +// reportStepError emits the GitHub Actions ##[error] annotation and records +// the error against the job so the job is reported as failed. +func reportStepError(ctx context.Context, err error) { + common.Logger(ctx).Errorf("##[error]%v", err) + common.SetJobError(ctx, err) +} + func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executor { steps := make([]common.Executor, 0) preSteps := make([]common.Executor, 0) @@ -32,7 +39,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo steps = append(steps, func(ctx context.Context) error { logger := common.Logger(ctx) if len(info.matrix()) > 0 { - logger.Infof("\U0001F9EA Matrix: %v", info.matrix()) + logger.Infof("Matrix: %v", info.matrix()) } return nil }) @@ -75,33 +82,36 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo preExec := step.pre() preSteps = append(preSteps, useStepLogger(rc, stepModel, stepStagePre, func(ctx context.Context) error { - logger := common.Logger(ctx) preErr := preExec(ctx) if preErr != nil { - logger.Errorf("%v", preErr) - common.SetJobError(ctx, preErr) + reportStepError(ctx, preErr) } else if ctx.Err() != nil { - logger.Errorf("%v", ctx.Err()) - common.SetJobError(ctx, ctx.Err()) + reportStepError(ctx, ctx.Err()) } return preErr })) stepExec := step.main() steps = append(steps, useStepLogger(rc, stepModel, stepStageMain, func(ctx context.Context) error { - logger := common.Logger(ctx) err := stepExec(ctx) if err != nil { - logger.Errorf("%v", err) - common.SetJobError(ctx, err) + reportStepError(ctx, err) } else if ctx.Err() != nil { - logger.Errorf("%v", ctx.Err()) - common.SetJobError(ctx, ctx.Err()) + reportStepError(ctx, ctx.Err()) } return nil })) - postExec := useStepLogger(rc, stepModel, stepStagePost, step.post()) + postFn := step.post() + postExec := useStepLogger(rc, stepModel, stepStagePost, func(ctx context.Context) error { + err := postFn(ctx) + if err != nil { + reportStepError(ctx, err) + } else if ctx.Err() != nil { + reportStepError(ctx, ctx.Err()) + } + return err + }) if postExecutor != nil { // run the post executor in reverse order postExecutor = postExec.Finally(postExecutor) @@ -196,7 +206,7 @@ func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success boo jobResultMessage = "failed" } - logger.WithField("jobResult", jobResult).Infof("\U0001F3C1 Job %s", jobResultMessage) + logger.WithField("jobResult", jobResult).Infof("Job %s", jobResultMessage) } func setJobOutputs(ctx context.Context, rc *RunContext) { diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 41302650..9a82ebe3 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -730,7 +730,7 @@ func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { jobType, jobTypeErr := job.Type() if runJobErr != nil { - return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, runJobErr) + return false, fmt.Errorf("if-expression %q evaluation failed: %s", job.If.Value, runJobErr) } if jobType == model.JobTypeInvalid { diff --git a/act/runner/step.go b/act/runner/step.go index d4ee50ac..8a241acf 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -107,7 +107,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo if strings.Contains(stepString, "::add-mask::") { stepString = "add-mask command" } - logger.Infof("\u2B50 Run %s %s", stage, stepString) + logger.Infof("Run %s %s", stage, stepString) // Prepare and clean Runner File Commands actPath := rc.JobContainer.GetActPath() @@ -158,7 +158,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo err = executor(timeoutctx) if err == nil { - logger.WithField("stepResult", stepResult.Outcome).Infof(" \u2705 Success - %s %s", stage, stepString) + logger.WithField("stepResult", stepResult.Outcome).Infof("Success - %s %s", stage, stepString) } else { stepResult.Outcome = model.StepStatusFailure @@ -169,6 +169,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo } if continueOnError { + logger.Errorf("##[error]%v", err) logger.Infof("Failed but continue next step") err = nil stepResult.Conclusion = model.StepStatusSuccess @@ -176,7 +177,9 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo stepResult.Conclusion = model.StepStatusFailure } - logger.WithField("stepResult", stepResult.Outcome).Errorf(" \u274C Failure - %s %s", stage, stepString) + // Infof: Errorf entries are promoted to the user log by the reporter, + // which would duplicate the ##[error] annotation emitted elsewhere. + logger.WithField("stepResult", stepResult.Outcome).Infof("Failure - %s %s", stage, stepString) } // Process Runner File Commands orgerr := err @@ -268,7 +271,7 @@ func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage) runStep, err := EvalBool(ctx, rc.NewStepExpressionEvaluator(ctx, step), expr, defaultStatusCheck) if err != nil { - return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", expr, err) + return false, fmt.Errorf("if-expression %q evaluation failed: %s", expr, err) } return runStep, nil @@ -284,7 +287,7 @@ func isContinueOnError(ctx context.Context, expr string, step step, _ stepStage) continueOnError, err := EvalBool(ctx, rc.NewStepExpressionEvaluator(ctx, step), expr, exprparser.DefaultStatusCheckNone) if err != nil { - return false, fmt.Errorf(" \u274C Error in continue-on-error-expression: \"continue-on-error: %s\" (%s)", expr, err) + return false, fmt.Errorf("continue-on-error expression %q evaluation failed: %s", expr, err) } return continueOnError, nil