fix: only treat a crash with a cascade frame as non-transient - #147
Merged
Merged
Conversation
The crash classifier introduced in #146 flagged any act exit carrying a Go crash signature as a non-transient hard failure. On CI run 27480827218 this hard-failed Hotfix_Refusal_Guards on a bare goroutine dump from act's own monitorJobCancellation teardown watchdog (under cancel-in-progress): no panic/fatal/signal trigger and zero cascade frames, just nektos/act frames. That scenario passes locally with no retries, so it was an act-infra teardown flake that retry should absorb, not a real bug. Refine detectCrashSignature to classify a crash as non-transient only when BOTH a real runtime trigger (panic:/fatal error: at line start, or a signal SIG report) AND a real cascade stack frame are present. Drop the standalone goroutine-dump-header trigger. A non-zero act exit lacking either stays transient and retryable, preserving #146's benign-transient and real-job-failure paths. A genuine cascade-CLI panic still carries both a trigger and cmd/cascade or internal/... frames, so it is still caught and fast-failed. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The e2e crash classifier from #146 is over-aggressive and causes false CI reds. It flagged any act exit carrying a Go crash signature as a non-transient hard failure, including a bare goroutine-dump header with no real trigger.
On CI run 27480827218,
Hotfix_Refusal_Guardsstep 2 failed with a bare goroutine dump (goroutine 470 [wselect]:) emitted by act's ownmonitorJobCancellationteardown watchdog undercancel-in-progress: nopanic:/fatal error:/signal trigger line, and zerostablekernel/cascadeframes (only nektos/act frames). That scenario passes 4/4 locally with 0 retries, so it was a pure act-infra teardown flake that retry should absorb, not a real bug.Fix
Refine
detectCrashSignatureso a crash is classified non-transient only when BOTH hold:panic:orfatal error:at the start of a (trimmed) line, or asignal SIG(SEGV|ABRT|BUS|FPE|ILL)report. The standalone goroutine-dump-header trigger is removed (act prints full goroutine dumps routinely on cancellation teardown).stablekernel/cascade/(internal|cmd)...followed by a function/receiver identifier), so it does not match git-URL prose, stdlibinternal/poll, or vendored.../internal/...packages.A non-zero act exit lacking either condition stays transient and retryable, preserving #146's benign-transient path and the real-job-failure path. A genuine cascade-CLI panic still carries both a trigger and
cmd/cascade+internal/...frames, so it is still caught and fast-failed.Verification
e2e/harness/crash_test.gocover: panic+cascade frame (crash),[signal SIGSEGV]+cascade frame (crash), act-only panic (transient), bare actmonitorJobCancellationdump (transient), trigger-without-frame and frame-without-trigger (both transient), and git-URL prose (not a crash).go build ./...+go test ./...(root),go build/vet ./...+go test ./harness/...(e2e): all green.golangci-lint run ./...clean on both modules.