-
Notifications
You must be signed in to change notification settings - Fork 231
Description
Description
Critical compilation and GitHub API operations lack panic recovery mechanisms, creating a risk of complete workflow crashes from unexpected panics. A single unhandled panic in production could terminate an entire workflow run without proper error reporting.
Problem
Current State:
- 0 panic recovery implementations across the entire codebase
- Complex operations (GitHub API calls, workflow compilation, file I/O, JSON parsing) have no safety nets
- Panics in these paths crash workflows without useful error messages
Impact:
- Production incidents from unexpected panics (nil pointers, type assertions, index out of bounds)
- Poor user experience with cryptic crash messages
- No error context or stack traces for debugging
Suggested Changes
Implement panic recovery with defer recover() in critical paths:
Files to Update
-
pkg/workflow/compiler*.go- Main compilation entry pointsCompile(),CompileWorkflow(),CompileAction()functions
-
pkg/cli/*_command.go- Interactive CLI commandstrial_command.go- Trial workflow execution (48 unformatted errors)add_interactive.go- Interactive workflow creation (107 unformatted errors)init.go- Repository initialization (100 unformatted errors)
-
GitHub API client operations - Network-dependent operations
Implementation Pattern
func CriticalOperation() (err error) {
defer func() {
if r := recover(); r != nil {
// Convert panic to error with stack trace
err = fmt.Errorf("panic during operation: %v\\nstack trace:\\n%s",
r, string(debug.Stack()))
// Log for debugging
if log.Enabled() {
log.Printf("Recovered from panic: %v", r)
}
}
}()
// Existing operation code...
return nil
}Test Coverage
Add test cases for common panic scenarios:
- Nil pointer dereference
- Index out of bounds
- Type assertion failures
- Map access on nil map
Example:
func TestCompiler_PanicRecovery(t *testing.T) {
tests := []struct {
name string
setupPanic func()
wantErr string
}{
{
name: "nil pointer panic",
setupPanic: func() { var x *int; _ = *x },
wantErr: "panic during",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := recoverableOperation(tt.setupPanic)
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
})
}
}Success Criteria
- Panic recovery added to main compiler entry points
- Stack traces included in error messages (using
runtime/debug.Stack()) - Panics logged to debug logger for investigation
- Structured errors work with console formatting
- Test cases verify panic recovery for common scenarios
- All tests pass:
make test-unit
Files Affected
pkg/workflow/compiler.gopkg/workflow/compiler_yaml.gopkg/cli/trial_command.gopkg/cli/add_interactive.gopkg/cli/init.go
Priority
High - Prevents production crashes and improves error reporting
Source
Extracted from Error Experience Engineering discussion #12265
Analysis Context:
- Only 4.4% (69/1,581) of error output sites use proper console formatting
- Zero panic recovery implementations found
- Top 5 CLI files contain 388 unformatted error outputs (25% of total)
AI generated by Discussion Task Miner - Code Quality Improvement Agent
- expires on Feb 18, 2026, 5:18 PM UTC