Skip to content

Consolidate error types: replace Result<_, String> with typed enums in update.rs #27

@codesoda

Description

@codesoda

Summary

update.rs uses Result<_, String> throughout with ~18 map_err(|e| format!(...)) calls, while every other module uses typed error enums (CommandError, ExecutorError, ConfigError). This prevents callers from pattern-matching on update errors and is inconsistent with the rest of the codebase.

Severity: S2 (Medium) | Confidence: 1.0 | Blast radius: Low

Technical Details

update.rs — untyped string errors:

// src/update.rs — every function signature uses -> Result<_, String>
// Lines: 52, 76, 138, 149, 185, 218, 240, 265, 327, 361, 369, 399
pub fn check_for_update(...) -> Result<Option<UpdateInfo>, String>
pub fn run_update_install(...) -> Result<(), String>
// etc.

// Error construction pattern repeated ~18 times:
.map_err(|e| format!("failed to parse version: {e}"))?
.map_err(|e| format!("HTTP request failed: {e}"))?
.map_err(|e| format!("failed to extract archive: {e}"))?

Rest of codebase — typed error enums:

// src/command.rs
pub enum CommandError { NonZeroExit { .. }, TimedOut { .. }, SpawnFailed { .. } }

// src/executor.rs
pub enum ExecutorError { Provider(ProviderError), Command(CommandError), ... }

// src/config.rs
pub enum ConfigError { Io(io::Error), Parse(toml::de::Error), ... }

Also affected: validate_skip_cmds and validate_skip_readiness in src/command.rs:80,111 return Result<(), String>.

Proposed Fix

  1. Create an UpdateError enum in src/update.rs:
    #[derive(Debug, thiserror::Error)]
    pub enum UpdateError {
        #[error("HTTP request failed: {0}")]
        Http(#[from] reqwest::Error),
        #[error("failed to parse version: {0}")]
        VersionParse(String),
        #[error("checksum mismatch: expected {expected}, got {actual}")]
        ChecksumMismatch { expected: String, actual: String },
        #[error("failed to extract archive: {0}")]
        Extract(#[from] std::io::Error),
        #[error("self-replace failed: {0}")]
        Replace(String),
    }
  2. Replace all ~18 map_err(|e| format!(...)) with typed variants or ? operator
  3. Migrate validate_skip_cmds / validate_skip_readiness in command.rs from Result<(), String> to a ValidationError type or fold into ConfigError

Estimated effort: ~4 hours

Related

  • Part of refactor Bundle 4: Error Type Consolidation

🔍 Found by vibe-code-audit — automated codebase audit skill for Claude Code.

Metadata

Metadata

Assignees

No one assigned

    Labels

    auditFound via automated code auditrefactorCode structure and architecture improvements

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions