Skip to content

[Code Quality] Standardize error wrapping across compiler files with %w format verb #14424

@github-actions

Description

@github-actions

Error handling practices vary inconsistently across compiler files, with some files properly using %w for error wrapping and others not wrapping at all. This breaks error chains and makes debugging compilation failures significantly harder.

Problem

Current inconsistency:

  • compiler_jobs.go: 16 instances of fmt.Errorf(..., %w) ✅ (Excellent)
  • compiler_activation_jobs.go: 4 out of 13 fmt.Errorf calls use %w (31% coverage)
  • compiler.go: 0 instances of %w wrapping ❌ (Uses formatCompilerError() which doesn't wrap)
  • compiler_yaml.go: 3 instances of %w wrapping
  • compiler_yaml_main_job.go: 1 instance of %w wrapping

Why this matters:

  • Broken error chains make it impossible to use errors.Is() and errors.As()
  • Lost error context makes debugging compilation failures harder
  • Inconsistent practices confuse developers and reduce code quality
  • Go 1.13+ best practice is to always wrap errors with %w

Impact Examples

❌ Current (No Wrapping):

// compiler.go - formatCompilerError doesn't wrap
func formatCompilerError(filePath, errType, message string, cause error) error {
    formatted := console.FormatError(...)
    return errors.New(formatted)  // Original 'cause' is lost!
}

Result: When compilation fails, we see a generic error message but lose the root cause (e.g., file not found, YAML parse error).

✅ After Fix (With Wrapping):

func formatCompilerError(filePath, errType, message string, cause error) error {
    formatted := console.FormatError(...)
    if cause != nil {
        return fmt.Errorf("%s: %w", formatted, cause)  // Preserves chain
    }
    return errors.New(formatted)
}

Result: Full error chain is preserved, allowing errors.Unwrap() to trace back to root cause.

Suggested Changes

1. Update formatCompilerError() in compiler.go (HIGH)

// Before:
func formatCompilerError(filePath, errType, message string, cause error) error {
    formatted := console.FormatError(...)
    return errors.New(formatted)
}

// After:
func formatCompilerError(filePath, errType, message string, cause error) error {
    formatted := console.FormatError(...)
    if cause != nil {
        return fmt.Errorf("%s: %w", formatted, cause)
    }
    return errors.New(formatted)
}

2. Add wrapping to compiler_activation_jobs.go (MEDIUM)

Audit 13 fmt.Errorf calls, update 9 missing %w:

// Before:
if err := c.extractPreActivationCustomFields(...); err != nil {
    return nil, fmt.Errorf("failed to extract custom fields from pre-activation job: %s", err)
}

// After:
if err := c.extractPreActivationCustomFields(...); err != nil {
    return nil, fmt.Errorf("failed to extract custom fields from pre-activation job: %w", err)
}

3. Add wrapping to compiler_yaml.go (LOW)

Ensure all error returns use %w (currently 3, may have more to fix).

4. Add wrapping to compiler_yaml_main_job.go (LOW)

Increase from 1 to all error paths.

Files Affected

  • pkg/workflow/compiler.go - Update formatCompilerError() signature
  • pkg/workflow/compiler_activation_jobs.go - Update 9 error returns
  • pkg/workflow/compiler_yaml.go - Audit and update error returns
  • pkg/workflow/compiler_yaml_main_job.go - Audit and update error returns

Implementation Approach

Phase 1: Fix formatCompilerError() (30 min)

  1. Update function to accept and wrap cause errors with %w
  2. Test with a failing compilation to verify error chain preserved
  3. Update all call sites if signature changes

Phase 2: Audit Each File (1-2 hours)

# Find all fmt.Errorf without %w in compiler files
grep -n 'fmt.Errorf' pkg/workflow/compiler*.go | grep -v '%w'

# For each occurrence:
# 1. Verify it should wrap an error
# 2. Change format string from %s or %v to %w
# 3. Ensure wrapped error is the last argument

Phase 3: Verify (30 min)

  1. Run make test-unit - ensure tests pass
  2. Run make lint - check for issues
  3. Test error chains with integration test:
    func TestErrorChaining(t *testing.T) {
        err := CompileWorkflow("invalid.md")
        var pathErr *os.PathError
        require.True(t, errors.As(err, &pathErr), "Should unwrap to PathError")
    }

Success Criteria

  • formatCompilerError() wraps cause errors with %w
  • All compiler files consistently use %w for error wrapping
  • All tests pass (make test-unit)
  • Error chains can be unwrapped with errors.Is() and errors.As()
  • Code formatted and linted (make agent-finish)

Priority

MEDIUM - Not breaking functionality, but impacts debugging experience and code quality.

Estimated Effort

2-3 hours total (mostly audit and mechanical changes)

Source

Extracted from discussions:

Both reports identified error wrapping as a Medium Priority improvement across multiple compiler files.

References

AI generated by Discussion Task Miner - Code Quality Improvement Agent

  • expires on Feb 8, 2026, 9:05 PM UTC

Metadata

Metadata

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions