Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 29, 2025

Error types between Create and Destroy commands used inconsistent patterns: tuple vs struct wrapping, missing actionable tips, and no logical grouping.

Changes

Error Structure Template (docs/contributing/error-handling.md)

  • Added comprehensive template showing:
    • Error grouping with section comments
    • Struct-style variants with named fields
    • Actionable tips in error messages
    • .help() method implementation patterns

Create Command Errors

  • Converted tuple-style to struct-style for context preservation:
    // Before
    ConfigValidationFailed(#[source] source)
    
    // After
    ConfigValidationFailed {
        #[source]
        source: CreateConfigError,
    }
  • Added tips to all variants:
    • ConfigParsingFailed: "Validate JSON syntax with: jq . < {path}"
    • CommandFailed: "Check logs with --log-output file-and-stderr"
    • Others similarly enhanced
  • Added section comments:
    • Configuration File Errors
    • Command Execution Errors
    • Template Generation Errors

Destroy Command Errors

  • Added section comments for logical grouping (Validation → Repository → Operations)
  • Reordered variants to match grouping

Updated Call Sites

  • Updated error construction: Error(source)Error { source }
  • Updated pattern matching: Error(_)Error { .. }
  • Files: template.rs, environment.rs, config_loader.rs, tests/integration.rs

Result

All errors now follow uniform patterns:

  • Named fields for better pattern matching
  • Actionable tips in messages
  • Logical grouping with section comments
  • Consistent #[source] usage for error chains
Original prompt

This section details on the original issue you should resolve

<issue_title>Improve Error Consistency Between Commands</issue_title>
<issue_description>Parent Issue: #63
Type: 🔨 Structural Improvement
Impact: 🟢🟢 Medium
Effort: 🔵🔵 Medium
Priority: P1

Problem

Error structures differ between commands without clear reason:

// Create errors
pub enum CreateSubcommandError {
    ConfigFileNotFound { path: PathBuf },
    InvalidJson { path: PathBuf, source: serde_json::Error },
    TemplateGenerationFailed(CreateConfigError),
}

// Destroy errors
pub enum DestroySubcommandError {
    InvalidEnvironmentName { name: String, source: ... },
    CommandExecutionFailed { source: ... },
    RepositoryAccessFailed { data_dir: String, reason: String },
}

Some use structured fields consistently, others mix approaches.

Proposed Solution

Establish consistent patterns:

  1. Always include relevant context (paths, names, etc.)
  2. Always use #[source] for error chains
  3. Always include brief tips in error messages
  4. Group related error types consistently

Example standardized structure:

#[derive(Debug, Error)]
pub enum CreateSubcommandError {
    // File-related errors
    #[error("Configuration file not found: {path}
Tip: Check that the file path is correct: ls -la {path}")]
    ConfigFileNotFound {
        path: PathBuf,
    },

    #[error("Failed to parse configuration file '{path}' as {format}: {source}
Tip: Validate JSON syntax with: jq . < {path}")]
    ConfigParsingFailed {
        path: PathBuf,
        format: String,
        #[source]
        source: serde_json::Error,
    },

    // Command execution errors
    #[error("Create command execution failed: {source}
Tip: Check logs for detailed error information")]
    CommandExecutionFailed {
        #[source]
        source: Box<dyn std::error::Error + Send + Sync>,
    },

    // ... more errors
}

Benefits

  • ✅ Predictable error patterns across all commands
  • ✅ Better error messages with actionable tips
  • ✅ Proper error chain preservation with #[source]
  • ✅ Easier to handle errors in tests and calling code

Implementation Checklist

Phase 1: Document standards

  • Document error patterns in error handling guide
  • Create error structure template
  • Add examples of good vs bad error definitions

Phase 2: Standardize create errors

  • Review all CreateSubcommandError variants
  • Add missing context fields (paths, names, etc.)
  • Add #[source] attributes where needed
  • Add actionable tips to error messages
  • Group related errors together

Phase 3: Standardize destroy errors

  • Review all DestroySubcommandError variants
  • Add missing context fields
  • Add #[source] attributes where needed
  • Add actionable tips to error messages
  • Group related errors together

Phase 4: Validation

  • Verify error messages are helpful and consistent
  • Test error chain preservation
  • Update tests that check error types
  • Run all tests
  • Run linter

Acceptance Criteria

  • All error types use consistent structure
  • All errors include relevant context fields
  • All errors use #[source] for error chains
  • All error messages include actionable tips
  • Error patterns are documented
  • All tests pass: cargo test presentation::commands
  • Pre-commit checks pass: ./scripts/pre-commit.sh
  • Code follows project conventions

Related Documentation

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.

Copilot AI and others added 2 commits October 29, 2025 13:30
- Add error structure template to error handling guide
- Standardize Create command errors (add tips, convert to struct-style)
- Add error grouping comments to both Create and Destroy commands
- Update all error construction sites and test patterns
- All tests pass and linter checks succeed

Co-authored-by: josecelano <[email protected]>
Copilot AI changed the title [WIP] Improve error consistency between commands Standardize error structures across presentation commands Oct 29, 2025
Copilot AI requested a review from josecelano October 29, 2025 13:54
@josecelano josecelano marked this pull request as ready for review October 29, 2025 17:44
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

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

ACK 25826e6

@josecelano josecelano merged commit f09b0d5 into main Oct 29, 2025
48 checks passed
Copilot AI added a commit that referenced this pull request Oct 29, 2025
Copilot AI added a commit that referenced this pull request Oct 29, 2025
Copilot AI added a commit that referenced this pull request Oct 29, 2025
josecelano added a commit that referenced this pull request Oct 29, 2025
b4092b6 fix: [#90] add SKIP_AI_ENFORCEMENT to coverage steps (copilot-swe-agent[bot])
7653193 fix: [#90] add explicit permissions to coverage workflow (copilot-swe-agent[bot])
5afe9f8 feat: [#90] add coverage CI workflow (copilot-swe-agent[bot])
213b269 Initial plan (copilot-swe-agent[bot])

Pull request description:

  - [x] Add `SKIP_AI_ENFORCEMENT: 1` environment variable to coverage steps
  - [x] Applied to both `coverage-text` and `coverage-html` steps
  - [x] Matches pattern from `testing.yml` workflow
  - [x] Skips AI enforcement tests that require third-party tools (cargo machete, Ansible, OpenTofu)
  - [x] YAML linting passes

  <!-- START COPILOT CODING AGENT SUFFIX -->

  <details>

  <summary>Original prompt</summary>

  >
  > ----
  >
  > *This section details on the original issue you should resolve*
  >
  > <issue_title>Create coverage CI workflow</issue_title>
  > <issue_description>**Parent Epic**: #85
  >
  > ## Overview
  >
  > Create a GitHub Actions workflow to generate code coverage reports on every push and pull request. The workflow generates coverage in two formats: a text summary for immediate viewing in the workflow output, and an HTML report uploaded as an artifact for detailed inspection.
  >
  > **Note**: This issue does NOT include Codecov or other external coverage service integration.
  >
  > ## Goals
  >
  > - [ ] Generate coverage reports automatically in CI/CD for all branches
  > - [ ] Display coverage summary in workflow output (text format)
  > - [ ] Make detailed HTML coverage reports available as artifacts
  > - [ ] Add `.coverage/` directory to `.gitignore`
  >
  > ## Specifications
  >
  > ### Workflow Structure
  >
  > **Trigger Events**: Push and pull_request (following `testing.yml` pattern)
  >
  > **Job**: Single `coverage` job with these steps:
  >
  > 1. Checkout code - `actions/checkout@v4`
  > 2. Setup Rust toolchain - `dtolnay/rust-toolchain@stable`
  > 3. Enable caching - `Swatinem/rust-cache@v2`
  > 4. Install cargo-llvm-cov - `taiki-e/install-action@v2`
  > 5. Generate text coverage - `cargo cov`
  > 6. Generate HTML report - `cargo cov-html`
  > 7. Upload HTML artifact - `actions/upload-artifact@v4`
  >
  > ### Example Workflow
  >
  > ```yaml
  > name: Coverage
  >
  > on:
  >   push:
  >   pull_request:
  >
  > env:
  >   CARGO_TERM_COLOR: always
  >
  > jobs:
  >   coverage:
  >     name: Coverage Report
  >     runs-on: ubuntu-latest
  >
  >     steps:
  >       - id: checkout
  >         name: Checkout Repository
  >         uses: actions/checkout@v4
  >
  >       - id: setup
  >         name: Setup Toolchain
  >         uses: dtolnay/rust-toolchain@stable
  >
  >       - id: cache
  >         name: Enable Workflow Cache
  >         uses: Swatinem/rust-cache@v2
  >
  >       - id: install-llvm-cov
  >         name: Install cargo-llvm-cov
  >         uses: taiki-e/install-action@v2
  >         with:
  >           tool: cargo-llvm-cov
  >
  >       - id: coverage-text
  >         name: Generate Text Coverage Summary
  >         run: cargo cov
  >
  >       - id: coverage-html
  >         name: Generate HTML Coverage Report
  >         run: cargo cov-html
  >
  >       - id: upload-coverage
  >         name: Upload HTML Coverage Report
  >         uses: actions/upload-artifact@v4
  >         with:
  >           name: coverage-html-report
  >           path: target/llvm-cov/html/
  >           retention-days: 30
  > ```
  >
  > ## Implementation Plan
  >
  > ### Phase 1: Update .gitignore (5 minutes)
  >
  > - [ ] Add `.coverage/` directory to `.gitignore` with comment
  >
  > ### Phase 2: Create Workflow (15 minutes)
  >
  > - [ ] Create `.github/workflows/coverage.yml`
  > - [ ] Add trigger events and environment variables
  > - [ ] Set up job structure
  >
  > ### Phase 3: Configure Toolchain (10 minutes)
  >
  > - [ ] Add Rust toolchain setup, caching, and cargo-llvm-cov installation
  >
  > ### Phase 4: Generate Reports (15 minutes)
  >
  > - [ ] Add text and HTML coverage generation steps
  >
  > ### Phase 5: Upload Artifact (10 minutes)
  >
  > - [ ] Add artifact upload step with 30-day retention
  >
  > ### Phase 6: Testing (15 minutes)
  >
  > - [ ] Test workflow on a feature branch
  > - [ ] Verify text output and HTML artifact availability
  >
  > ## Acceptance Criteria
  >
  > - [ ] `.coverage/` directory added to `.gitignore`
  > - [ ] Workflow file exists at `.github/workflows/coverage.yml`
  > - [ ] Workflow triggers on push and pull_request
  > - [ ] Text coverage summary visible in workflow output
  > - [ ] HTML coverage artifact uploaded with 30-day retention
  > - [ ] Workflow does NOT block CI/CD on coverage failures
  >
  > ## Notes
  >
  > - Uses `cov` and `cov-html` aliases from `.cargo/config.toml`
  > - Follows patterns from existing `testing.yml` workflow
  > - Non-blocking by design - coverage is informational only
  > - Future enhancement: External coverage service integration can be added later</issue_description>
  >
  > ## Comments on the Issue (you are @copilot in this section)
  >
  > <comments>
  > </comments>
  >

  </details>

  - Fixes #87

  <!-- START COPILOT CODING AGENT TIPS -->
  ---

  💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

ACKs for top commit:
  josecelano:
    ACK b4092b6

Tree-SHA512: f5ee09387a3b4de3183ec90dcccebe771b8925722be123dd1715bfdb6fa3058fa5958548a2893ead822256cd33879e69438e59165866a50736a18123569c44d4
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.

Improve Error Consistency Between Commands

2 participants