Skip to content

feat: honest error reporting, prune-skip summary, extraction fixes - #80

Merged
MaxandreOgeret merged 7 commits into
mainfrom
feat/honest-user-errors
Jul 2, 2026
Merged

feat: honest error reporting, prune-skip summary, extraction fixes#80
MaxandreOgeret merged 7 commits into
mainfrom
feat/honest-user-errors

Conversation

@MaxandreOgeret

@MaxandreOgeret MaxandreOgeret commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes surfaced by a full adversarial code review of the repository.

  • New ColliderUserError(message, exit_code) handled at the entrypoint: usage and user-environment failures (malformed/unreadable collider.json/collider.lock, undeletable artifacts, invalid regex, missing -- separator) now exit with accurate os.EX_* codes and a clean message instead of the "probably a bug within collider" banner.
  • Lock read errors are labeled accurately (EX_IOERR) instead of "malformed", and the colliderfile fallback catch is narrowed. Closes project_state: narrow broad except in managed_package_names (read errors mislabeled, colliderfile swallowed) #52
  • remove --prune / prune end skipped runs with a trailing greppable summary line, exit code unchanged (EX_OK). Closes pkg remove --prune: print end-of-run summary when prune is skipped (no lockfile) #46
  • Tar extraction no longer breaks on interpreters without the PEP 706 filter= backport; corrupt cached releases.json is treated as a cache miss on all paths; status logs a warning when resolution fails instead of silently mislabeling transitive wraps; dead name-only Requirement.__eq__/__hash__ removed.

Known exemption: the TOCTOU FileNotFoundError race named in #52 (lock deleted between exists() and open) still routes to the bug handler; the window is negligible and all callers guard with exists(). Diagnostic follow-up for CRITICAL-level logs on best-effort paths: #79.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines.
  • My commits follow Conventional Commits.
  • Tests pass (uv run pytest).
  • Format, lint, type, and static checks pass (uv run ruff format --check ., uv run ruff check ., uv run ty check collider, uv run pylint --rcfile=pyproject.toml ./collider).

AI disclosure (Tick all that apply)

  • AI tools were used for part or all of this change.
    • A human (not an AI agent) has reviewed every line, understands the change, and takes full responsibility for what is submitted.

Introduce ColliderUserError carrying an os.EX_* exit code, handled at the
entrypoint so usage and environment problems no longer surface as the
internal-bug banner with exit 1.

- FileModelInterface.from_path maps parse/validation/deserialization
  failures to EX_DATAERR and read errors to EX_IOERR, fixing every
  subcommand that loads collider.json or collider.lock in one place.
- Setup (missing "--" separator) and pkg search (invalid regex) exit
  EX_USAGE cleanly.
- remove_installed_artifacts raises EX_IOERR on undeletable artifacts;
  extracted trees are removed before the wrap so a failed removal stays
  retryable.
- Add/Upgrade/Install install blocks handle OSError instead of only
  FileNotFoundError, and unreadable wrap files no longer crash
  pkg info or pkg upgrade.
- managed_package_names and detect_locked_wrap_drift stop wrapping lock
  read errors into a misleading "malformed" ValueError.
- status logs a warning when version resolution fails instead of
  silently listing transitive wraps as untracked.

Known exemption: the TOCTOU FileNotFoundError race (lock deleted
between exists() and open) still routes to the bug handler; the window
is negligible and callers guard with exists().
When remove --prune or prune cannot run because collider.lock is absent
or unreadable, the command now ends with a single greppable line:
'prune skipped: <reason>; run "collider lock".' Exit code stays EX_OK,
since pruning a project that legitimately has no lock is valid.

The lockfile-refresh warning in remove moved ahead of the prune step so
the summary is always the last log line.
- Pass tarfile's filter kwarg only when the PEP 706 backport is present
  (hasattr tarfile.data_filter); Python 3.10.0-3.10.11 and 3.11.0-3.11.3
  raised TypeError on every tar extraction otherwise.
- Treat an unreadable or corrupt cached releases.json as a cache miss:
  within-TTL corruption refetches from the network, and the
  network-failure fallback re-raises the network error instead of
  masking it with a JSON parse crash.
- Drop Requirement/Candidate custom __eq__/__hash__: resolvelib
  identifies requirements via identify() (name), never by hash, and the
  frozen dataclass equality is field-complete; same-name requirements
  with different constraints no longer collapse in sets.
- Use errno.EXDEV over the literal 18 and missing_ok on the corrupt
  archive unlink.
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.42975% with 14 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
collider/subcommand/pkg/Upgrade.py 22.22% 7 Missing ⚠️
collider/subcommand/pkg/Add.py 25.00% 3 Missing ⚠️
collider/errors.py 66.66% 1 Missing and 1 partial ⚠️
collider/cache.py 66.66% 0 Missing and 1 partial ⚠️
collider/subcommand/pkg/Prune.py 83.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

- Validate WrapDB releases.json is a JSON object after fetch; raise
  ColliderUserError(EX_DATAERR) instead of an opaque RuntimeError so a
  null/non-dict 200 body is reported honestly.
- Extend the final guard to cover non-dict cached/offline releases.
- Add contract tests asserting install returns EX_IOERR when cache
  preparation or wrap-file writing fails with a permission error.
- Add a test for the stale-cache fallback warning and for the null-body
  data error path.
@MaxandreOgeret
MaxandreOgeret force-pushed the feat/honest-user-errors branch from 7be40c7 to abfecf4 Compare July 2, 2026 14:00
- Repo: re-raise ColliderUserError from config load so the carried exit
  code reaches the entrypoint instead of being flattened to EX_DATAERR.
- Prune: return the carried exit code on a corrupt lockfile so direct
  `collider pkg prune` fails honestly instead of exiting EX_OK.
- Wrap: offline corrupt/missing cache raises ColliderUserError(EX_DATAERR)
  instead of ValueError, matching the non-object cache path; use bare raise.
- resolver: narrow the scan-failure catch to FileNotFoundError so real
  permission or disk-space errors propagate rather than degrading to an
  empty dependency list.
- project_state: tolerate a lockfile vanishing between exists() and open().
- errors: guard ColliderUserError against a zero exit code.
- Info: map UnicodeDecodeError to EX_DATAERR and OSError to EX_IOERR.
- config: keep skipping a repo that fails to load so one bad repo does not
  abort the whole CLI; the warning remains the user-facing signal.
@MaxandreOgeret
MaxandreOgeret force-pushed the feat/honest-user-errors branch from 1f595fa to 1d78a0a Compare July 2, 2026 14:57
A PermissionError or ENOSPC while staging a source for dependency
scanning propagated bare through resolvelib into install/lock/add,
whose handlers catch only resolver exceptions, and surfaced as the
internal-bug banner. Map it to ColliderUserError(EX_IOERR) at the scan
site so strict callers exit cleanly while best-effort callers keep
degrading gracefully.

Also name the cache file in the malformed cached-releases error and
document why remove --prune ignores run_prune's exit code (issue #46).
@MaxandreOgeret
MaxandreOgeret merged commit 47c79a5 into main Jul 2, 2026
18 checks passed
@MaxandreOgeret
MaxandreOgeret deleted the feat/honest-user-errors branch July 2, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant