Skip to content

Fix error propagation in compiler: return errors instead of swallowing them#14301

Merged
pelikhan merged 2 commits intomainfrom
copilot/standardize-error-wrapping
Feb 7, 2026
Merged

Fix error propagation in compiler: return errors instead of swallowing them#14301
pelikhan merged 2 commits intomainfrom
copilot/standardize-error-wrapping

Conversation

Copy link
Contributor

Copilot AI commented Feb 7, 2026

Two locations in the compiler were logging errors without returning them, breaking error propagation through the call stack.

Changes

  • compiler_yaml_main_job.go:78: json.Marshal failure now returns error instead of logging warning
  • compiler_activation_jobs.go:437: serializeMessagesConfig failure now returns error instead of logging warning

Both errors are properly wrapped with %w for context propagation.

Before

repoImportsJSON, err := json.Marshal(data.RepositoryImports)
if err != nil {
    compilerYamlLog.Printf("Warning: failed to marshal repository imports: %v", err)
} else {
    fmt.Fprintf(yaml, "          GH_AW_REPOSITORY_IMPORTS: '%s'\n", string(repoImportsJSON))
}

After

repoImportsJSON, err := json.Marshal(data.RepositoryImports)
if err != nil {
    return fmt.Errorf("failed to marshal repository imports for merge step: %w", err)
}
fmt.Fprintf(yaml, "          GH_AW_REPOSITORY_IMPORTS: '%s'\n", string(repoImportsJSON))

Note: Other fmt.Errorf calls without %w in compiler files are validation errors creating new errors, not wrapping existing ones. No changes needed there.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Code Quality] Standardize error wrapping across compiler files</issue_title>
<issue_description>## Description

Compiler files show inconsistent error wrapping patterns. Some files use fmt.Errorf with %w for proper error chaining (excellent), while others have gaps that make debugging compilation failures more difficult.

Current State

Error wrapping inconsistency across files:

File Error Wraps Coverage Status
compiler_jobs.go 16/16 100% ✅ Excellent
compiler_yaml.go 3/3 100% ✅ Excellent
compiler_activation_jobs.go 4/9 44% ⚠️ Needs work
compiler_yaml_main_job.go 1/2 50% ⚠️ Needs work

Examples of issues:

  1. compiler_yaml_main_job.go (line 66):

    // Current - error only logged, not returned
    if err != nil {
        compilerYamlLog.Printf("Warning: failed to marshal repository imports: %v", err)
    }
    
    // Should be
    if err != nil {
        return fmt.Errorf("failed to marshal repository imports for merge step: %w", err)
    }
  2. compiler_activation_jobs.go:

    • 9 fmt.Errorf calls total
    • Only 4 use %w for wrapping (44% coverage)
    • Missing context propagation for debugging

Impact

Without consistent error wrapping:

  • Harder to trace compilation failures through call stack
  • Loss of error context at higher levels
  • Debugging requires more time and effort
  • Reduced error actionability for users

Suggested Changes

  1. Audit all compiler files for error handling:

    grep -rn "return.*fmt.Errorf" pkg/workflow/compiler*.go | grep -v "%w"
  2. Fix missing error wrapping - Ensure all error returns use %w:

    return fmt.Errorf("failed to (action): %w", err)
  3. Return errors instead of only logging - Don't swallow errors:

    // Bad
    if err != nil {
        log.Printf("Warning: %v", err)
    }
    
    // Good  
    if err != nil {
        return fmt.Errorf("context: %w", err)
    }

Files Affected

Priority order (most gaps to fewest):

  1. pkg/workflow/compiler_activation_jobs.go - 5 missing wraps
  2. pkg/workflow/compiler_yaml_main_job.go - 1 missing wrap

Success Criteria

  • All fmt.Errorf calls with underlying errors use %w wrapper
  • No errors are logged without being returned (unless explicitly intended)
  • Error messages include actionable context
  • Grep audit shows 100% error wrap coverage: grep -rn "return.*fmt.Errorf" pkg/workflow/compiler*.go | grep -v "%w" returns no results
  • All existing tests still pass after changes

Priority

High - Quick win with high impact on debugging experience

Estimated Effort

30-60 minutes

Source

Extracted from compiler quality analysis discussions:

Identified as immediate action in both reports due to impact on debugging.

AI generated by Discussion Task Miner - Code Quality Improvement Agent

  • expires on Feb 8, 2026, 5:17 AM UTC

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…logging

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Standardize error wrapping across compiler files Fix error propagation in compiler: return errors instead of swallowing them Feb 7, 2026
Copilot AI requested a review from pelikhan February 7, 2026 07:27
@pelikhan pelikhan marked this pull request as ready for review February 7, 2026 13:24
Copilot AI review requested due to automatic review settings February 7, 2026 13:24
@pelikhan pelikhan merged commit 9a62188 into main Feb 7, 2026
1 check passed
@pelikhan pelikhan deleted the copilot/standardize-error-wrapping branch February 7, 2026 13:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves compiler error propagation by returning (and wrapping) errors that were previously only logged as warnings, ensuring failures surface correctly to callers during workflow compilation.

Changes:

  • Return a wrapped error when json.Marshal(data.RepositoryImports) fails while generating main job steps.
  • Return a wrapped error when serializeMessagesConfig(...) fails while building the activation job.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/workflow/compiler_yaml_main_job.go Fail fast when repository imports JSON serialization fails during main job step generation.
pkg/workflow/compiler_activation_jobs.go Fail fast when safe-output messages config serialization fails during activation job construction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Quality] Standardize error wrapping across compiler files

3 participants