fix: classify a cancelled step as an interruption, not a failure (#1095)

`reportStepError` reported every step error as FAILURE, including a `context.Canceled` from a docker file-command read cancelled at job finalization — non-deterministic red CI. Classify `context.Canceled` as an interruption instead (deferring to the job context), so a genuine cancel reports cancelled and a stray teardown cancellation is ignored, never a failure.

---------

Co-authored-by: bircni <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1095
Reviewed-by: bircni <bircni@icloud.com>
This commit is contained in:
silverwind
2026-07-21 11:16:56 +00:00
parent 89467c9dd0
commit 0e8896c52a
2 changed files with 36 additions and 2 deletions

View File

@@ -283,3 +283,30 @@ func TestPostStepsContextDeadlinePreservesJobError(t *testing.T) {
require.NoError(t, postCtx.Err(), "post context must not carry the expired deadline") require.NoError(t, postCtx.Err(), "post context must not carry the expired deadline")
assert.ErrorIs(t, common.JobError(postCtx), assert.AnError, "the timeout job error must be preserved") assert.ErrorIs(t, common.JobError(postCtx), assert.AnError, "the timeout job error must be preserved")
} }
// reportStepError must treat a context.Canceled (e.g. a teardown-cancelled read) as an
// interruption, never a job failure.
func TestReportStepErrorTreatsCancelAsInterruption(t *testing.T) {
rc := &RunContext{}
// stray read cancellation while the job context is live: ignored, not a failure
live := common.WithJobErrorContainer(context.Background())
reportStepError(live, rc, context.Canceled)
require.NoError(t, common.JobError(live))
assert.False(t, rc.jobFailed)
assert.False(t, rc.jobCancelled)
// genuine job cancellation: recorded as cancelled, still not a failure
cancelled, cancel := context.WithCancel(common.WithJobErrorContainer(context.Background()))
cancel()
reportStepError(cancelled, rc, context.Canceled)
require.NoError(t, common.JobError(cancelled))
assert.False(t, rc.jobFailed)
assert.True(t, rc.jobCancelled)
// a real error still fails the job
failed := common.WithJobErrorContainer(context.Background())
reportStepError(failed, rc, assert.AnError)
require.ErrorIs(t, common.JobError(failed), assert.AnError)
assert.True(t, rc.jobFailed)
}

View File

@@ -10,6 +10,7 @@ import (
"context" "context"
"encoding/base64" "encoding/base64"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
@@ -56,9 +57,15 @@ type jobInfo interface {
result(result string) result(result string)
} }
// reportStepError emits the GitHub Actions ##[error] annotation and records // reportStepError records a step error so the job is reported failed — except a
// the error against the job so the job is reported as failed. // cancellation, which is an interruption, not a failure.
func reportStepError(ctx context.Context, rc *RunContext, err error) { func reportStepError(ctx context.Context, rc *RunContext, err error) {
if errors.Is(err, context.Canceled) {
// Defer to the job context: a genuine cancel reports cancelled, a stray teardown
// cancellation on a live ctx is ignored — never a step FAILURE.
rc.markInterrupted(ctx.Err())
return
}
common.Logger(ctx).Errorf("##[error]%v", err) common.Logger(ctx).Errorf("##[error]%v", err)
common.SetJobError(ctx, err) common.SetJobError(ctx, err)
rc.markFailed() rc.markFailed()