Map s3 not-found to storage.ErrNotFound in Get and Stat (closes #129) #142

Merged
clawbot merged 1 commits from issue-129-s3-errnotfound into next 2026-09-21 21:07:36 +02:00
Collaborator

Fixes #129.

The Storer interface documents that Get and Stat return
storage.ErrNotFound for a missing object. The file and rclone
backends honor this; the s3 backend did not. S3Storer.Get and Stat
returned the raw AWS SDK error, so errors.Is(err, storage.ErrNotFound)
was false on s3 and any caller branching on ErrNotFound behaved
differently depending on the backend.

Both methods now wrap ErrNotFound when the SDK reports a missing
object, and leave every other error untouched. The not-found detection
lives in a small exported s3.IsNotFound helper in internal/s3, which
HeadObject also now uses, so the SDK's two missing-object error types
(NoSuchKey from Get, NotFound from Head/Stat) have one definition
instead of two copies.

Tripwire for the reader: the mapping stays in the storage layer, not
in internal/s3, because internal/s3 must not import storage
(storage imports it). The new test spins up an in-process gofakes3
server and asserts a missing key maps to ErrNotFound through the real
client; it fails without the mapping (Get surfaces NoSuchKey, Stat
NotFound).

The issue notes a TestS3StorerMissingKeyDivergence from the
#66 tests; that test is not on
next, so this adds its own directly-asserting test rather than
tightening one that isn't here.

model: claude-opus-4-8

Fixes https://git.eeqj.de/sneak/vaultik/issues/129. The `Storer` interface documents that `Get` and `Stat` return `storage.ErrNotFound` for a missing object. The `file` and `rclone` backends honor this; the s3 backend did not. `S3Storer.Get` and `Stat` returned the raw AWS SDK error, so `errors.Is(err, storage.ErrNotFound)` was false on s3 and any caller branching on `ErrNotFound` behaved differently depending on the backend. Both methods now wrap `ErrNotFound` when the SDK reports a missing object, and leave every other error untouched. The not-found detection lives in a small exported `s3.IsNotFound` helper in `internal/s3`, which `HeadObject` also now uses, so the SDK's two missing-object error types (`NoSuchKey` from Get, `NotFound` from Head/Stat) have one definition instead of two copies. Tripwire for the reader: the mapping stays in the `storage` layer, not in `internal/s3`, because `internal/s3` must not import `storage` (`storage` imports it). The new test spins up an in-process gofakes3 server and asserts a missing key maps to `ErrNotFound` through the real client; it fails without the mapping (Get surfaces `NoSuchKey`, Stat `NotFound`). The issue notes a `TestS3StorerMissingKeyDivergence` from the https://git.eeqj.de/sneak/vaultik/issues/66 tests; that test is not on `next`, so this adds its own directly-asserting test rather than tightening one that isn't here. model: claude-opus-4-8
clawbot added 1 commit 2026-09-21 20:57:26 +02:00
S3Storer.Get and Stat returned the raw AWS SDK error for a missing
object, so errors.Is(err, storage.ErrNotFound) was false on the s3
backend while the file and rclone backends honored the Storer contract.
Callers that branch on ErrNotFound behaved differently per backend.

Both now wrap ErrNotFound when the SDK reports a missing object,
leaving every other error intact. The not-found detection is a small
exported s3.IsNotFound helper, also used by HeadObject so the two share
one definition. A test asserts a missing key maps to ErrNotFound on the
s3 backend; it fails without the mapping.

model: claude-opus-4-8
clawbot added the needs-review label 2026-09-21 20:57:34 +02:00
clawbot self-assigned this 2026-09-21 20:57:34 +02:00
Author
Collaborator

PASS

model: claude-opus-4-8

PASS model: claude-opus-4-8
clawbot merged commit 9ca962969a into next 2026-09-21 21:07:36 +02:00
clawbot deleted branch issue-129-s3-errnotfound 2026-09-21 21:07:37 +02:00
Sign in to join this conversation.