feat(download): add ErrorClass classifier next to DownloadCacheError (PSA-38) #6

Merged

nrupard merged 2 commits from feat/psa-38-download-error-classifier into main

2026-06-03 19:47:30 +02:00

nrupard commented

2026-06-03 19:40:31 +02:00

Owner

What

Give dunite-download a consumer-agnostic error classifier next to DownloadCacheError, mirroring the PSA-35 precedent (From<&BlobCacheError> for OciError). Adds ErrorClass { NotFound | Upstream | Local } and DownloadCacheError::class(&self) -> ErrorClass, exported from services.

Why

dunite-oci owns its classification, so a consumer maps a blob failure to the right status with one call and the policy lives next to the type. dunite-download had no equivalent: each consumer hand-rolled the upstream-vs-local match (Forgejo NotFound -> 404, Forgejo/Transport -> 502, Io/Store -> 500), with the risk of drift between consumers. bunyip PR #50 (BUNYIP-44) made that match exhaustive, but the policy still lived in the consumer.

Shape

dunite-download cannot return a consumer's domain AppError, and the vertical has no fixed wire format (unlike OCI, whose mapping is pinned by the distribution spec), so a neutral classification enum is correct: the vertical owns the classification policy, the consumer owns the status mapping (typically NotFound -> 404, Upstream -> 502, Local -> 500). ShaMismatch/SizeMismatch classify as Upstream (the upstream served bad bytes), matching dunite-oci's DigestMismatch -> Upstream.

Tests

class_maps_every_variant (exhaustive) plus two live regression tests (upstream_404_classifies_as_not_found, upstream_unreachable_classifies_as_upstream) next to the existing typed-error regression tests. Full workspace: fmt + clippy -D warnings clean, cargo test -p dunite-download 30 passed.

Follow-up

bunyip's download handler can replace its hand-rolled match with err.class() (separate bunyip-side issue).

#PSA-38

## What Give dunite-download a consumer-agnostic error classifier next to `DownloadCacheError`, mirroring the PSA-35 precedent (`From<&BlobCacheError> for OciError`). Adds `ErrorClass { NotFound | Upstream | Local }` and `DownloadCacheError::class(&self) -> ErrorClass`, exported from `services`. ## Why dunite-oci owns its classification, so a consumer maps a blob failure to the right status with one call and the policy lives next to the type. dunite-download had no equivalent: each consumer hand-rolled the upstream-vs-local match (Forgejo NotFound -> 404, Forgejo/Transport -> 502, Io/Store -> 500), with the risk of drift between consumers. bunyip PR #50 (BUNYIP-44) made that match exhaustive, but the policy still lived in the consumer. ## Shape dunite-download cannot return a consumer's domain `AppError`, and the vertical has no fixed wire format (unlike OCI, whose mapping is pinned by the distribution spec), so a neutral classification enum is correct: the vertical owns the classification policy, the consumer owns the status mapping (typically `NotFound` -> 404, `Upstream` -> 502, `Local` -> 500). ShaMismatch/SizeMismatch classify as `Upstream` (the upstream served bad bytes), matching dunite-oci's `DigestMismatch` -> `Upstream`. ## Tests `class_maps_every_variant` (exhaustive) plus two live regression tests (`upstream_404_classifies_as_not_found`, `upstream_unreachable_classifies_as_upstream`) next to the existing typed-error regression tests. Full workspace: fmt + clippy -D warnings clean, `cargo test -p dunite-download` 30 passed. ## Follow-up bunyip's download handler can replace its hand-rolled match with `err.class()` (separate bunyip-side issue). #PSA-38

nrupard added 1 commit

2026-06-03 19:40:31 +02:00

feat(download): add ErrorClass classifier next to DownloadCacheError (PSA-38)

Checks / fmt + clippy + test (pull_request) Successful in 18s

Details

5e22e5a77f

dunite-oci owns its cache-error classification (`From<&BlobCacheError> for OciError`, PSA-35) so a consumer maps a blob fetch failure to the right OCI status with one call and the policy lives next to the type. dunite-download had no equivalent: each consumer hand-rolled the upstream-vs-local match (Forgejo NotFound -> 404, Forgejo/Transport -> 502, Io/Store -> 500), so a second consumer could drift.

dunite-download cannot return a consumer's domain `AppError`, and the vertical has no fixed wire format (unlike OCI, whose mapping is pinned by the distribution spec), so a neutral classification enum is the right shape: the vertical owns the classification policy, the consumer owns the status mapping. Adds `ErrorClass { NotFound | Upstream | Local }` and `DownloadCacheError::class(&self) -> ErrorClass`, exported from `services`. ShaMismatch/SizeMismatch classify as Upstream (the upstream served bad bytes), matching dunite-oci's DigestMismatch -> Upstream.

Covered by an exhaustive `class_maps_every_variant` unit test plus two live regression tests (`upstream_404_classifies_as_not_found`, `upstream_unreachable_classifies_as_upstream`) next to the existing typed-error regression tests. bunyip's download handler can replace its hand-rolled match with `err.class()` as a follow-up on the bunyip side.

#PSA-38

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

nrupard added 1 commit

2026-06-03 19:45:55 +02:00

fix(download): enumerate Forgejo variants in class() so a new one breaks compile (PSA-38 review)

Checks / fmt + clippy + test (pull_request) Successful in 20s

Details

create-release / create-release (pull_request) Has been skipped

Details

76568c69cd

Code review: the Upstream arm matched `Forgejo(_)`, which silently buckets any future `ForgejoError` variant as Upstream. List the variants explicitly (`Http | Upstream | InvalidUrl`) so adding a `ForgejoError` variant forces a compile error and an intentional classification here, rather than a silent default. Behavior is unchanged for the current variants.

#PSA-38

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>