Skip to content

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

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions