S3Storer.Get/Stat return the raw SDK error, not storage.ErrNotFound #129

Closed
opened 2026-09-21 09:41:28 +02:00 by clawbot · 1 comment
Collaborator

Found while adding storage backend tests for #66 (tests only; not fixed inline per that issue).

The Storer interface documents that Get and Stat return storage.ErrNotFound when the object does not exist (internal/storage/storer.go:55 and :59). The file and rclone backends honor this. The s3 backend does not: S3Storer.Get and S3Storer.Stat return the raw AWS SDK error unmapped (internal/storage/s3.go), so errors.Is(err, storage.ErrNotFound) is false for a missing object on the s3 backend.

Impact: any caller that branches on storage.ErrNotFound behaves differently depending on the backend. internal/s3 already exposes HeadObject returning a not-found signal, so the mapping is available.

The divergence is pinned by TestS3StorerMissingKeyDivergence in internal/storage/s3_test.go; that test should be tightened to assert ErrNotFound once this is fixed.

Model: opus-4-8

Found while adding storage backend tests for https://git.eeqj.de/sneak/vaultik/issues/66 (tests only; not fixed inline per that issue). The `Storer` interface documents that `Get` and `Stat` return `storage.ErrNotFound` when the object does not exist (`internal/storage/storer.go:55` and `:59`). The `file` and `rclone` backends honor this. The `s3` backend does not: `S3Storer.Get` and `S3Storer.Stat` return the raw AWS SDK error unmapped (`internal/storage/s3.go`), so `errors.Is(err, storage.ErrNotFound)` is false for a missing object on the s3 backend. Impact: any caller that branches on `storage.ErrNotFound` behaves differently depending on the backend. `internal/s3` already exposes `HeadObject` returning a not-found signal, so the mapping is available. The divergence is pinned by `TestS3StorerMissingKeyDivergence` in `internal/storage/s3_test.go`; that test should be tightened to assert `ErrNotFound` once this is fixed. Model: opus-4-8
clawbot added this to the 1.0.0 milestone 2026-09-21 09:47:33 +02:00
Author
Collaborator

Fixed in #142.

S3Storer.Get and Stat now wrap storage.ErrNotFound when the SDK
reports a missing object, matching the file and rclone backends and
the Storer contract; all other errors pass through unchanged. The
detection is a small exported s3.IsNotFound helper, also used by
HeadObject. A test on the s3 backend asserts a missing key maps to
ErrNotFound and fails without the change.

The referenced TestS3StorerMissingKeyDivergence is not on next, so
the PR adds its own directly-asserting test instead of tightening one
that isn't present.

model: claude-opus-4-8

Fixed in https://git.eeqj.de/sneak/vaultik/pulls/142. `S3Storer.Get` and `Stat` now wrap `storage.ErrNotFound` when the SDK reports a missing object, matching the `file` and `rclone` backends and the `Storer` contract; all other errors pass through unchanged. The detection is a small exported `s3.IsNotFound` helper, also used by `HeadObject`. A test on the s3 backend asserts a missing key maps to `ErrNotFound` and fails without the change. The referenced `TestS3StorerMissingKeyDivergence` is not on `next`, so the PR adds its own directly-asserting test instead of tightening one that isn't present. model: claude-opus-4-8
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#129