Skip to content

Fix: Narrow INSTALL_ERROR_TYPES catch scope #516

@kasperjunge

Description

@kasperjunge

INSTALL_ERROR_TYPES in exceptions.py catches broadly:

INSTALL_ERROR_TYPES = (FileExistsError, AgrError, OSError, ValueError)

Catching OSError and ValueError broadly risks swallowing unexpected errors and weakens the value of the exception hierarchy.

Proposal

  1. Replace bare ValueError raises within install/sync paths with specific AgrError subclasses (e.g., InvalidHandleError, ConfigError)
  2. Narrow OSError to specific subclasses (PermissionError, FileNotFoundError) where the failure mode is known
  3. Audit all raise sites that currently raise ValueError within the install/sync code paths and convert to the appropriate AgrError subclass

Goal state

INSTALL_ERROR_TYPES = (FileExistsError, AgrError, PermissionError, FileNotFoundError)

Or ideally just (AgrError, FileExistsError) if all ValueError and OSError sites are converted.

Affected files

  • agr/exceptions.pyINSTALL_ERROR_TYPES definition
  • agr/fetcher.py — primary consumer of the catch tuple
  • agr/commands/sync.py — uses INSTALL_ERROR_TYPES in except clauses
  • agr/commands/add.py — uses INSTALL_ERROR_TYPES in except clauses
  • Various modules that raise ValueError in install paths

Context

Identified in architecture audit: docs/contributing/architecture-audit.md

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions