Map s3 not-found to storage.ErrNotFound in Get and Stat (closes #129)
The Storer interface documents that Get and Stat return storage.ErrNotFound for a missing object. The file and rclone backends did; the s3 backend returned the raw SDK error, so callers testing for ErrNotFound behaved differently on s3. S3Storer.Get and Stat now wrap ErrNotFound when the SDK reports a missing object and leave every other error untouched. The SDK reports a missing key two ways (NoSuchKey from Get, NotFound from Head); both are recognised in one helper, s3.IsNotFound, which HeadObject now also uses. The mapping lives in the storage package because internal/s3 cannot import it. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)
This commit was merged in pull request #142.
This commit is contained in:
@@ -25,6 +25,15 @@ release" is exactly the contradiction
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-09-21: Made the s3 storage backend report a missing object as
|
||||
`storage.ErrNotFound`, like the `file` and `rclone` backends and as the
|
||||
`Storer` interface documents. `S3Storer.Get` and `Stat` returned the raw
|
||||
AWS SDK error, so `errors.Is(err, storage.ErrNotFound)` was false on s3
|
||||
and callers branched differently per backend. Added a small `s3.IsNotFound`
|
||||
helper (reused by `HeadObject`) and a test that a missing key maps to
|
||||
`ErrNotFound`
|
||||
([issue #129](https://git.eeqj.de/sneak/vaultik/issues/129)).
|
||||
|
||||
- 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as
|
||||
corrupt. Its final blob-integrity check hashed the encrypted
|
||||
downloaded bytes with a single SHA256 and compared that to the blob
|
||||
|
||||
+13
-5
@@ -219,11 +219,7 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
|
||||
Key: aws.String(fullKey),
|
||||
})
|
||||
if err != nil {
|
||||
var (
|
||||
notFound *s3types.NotFound
|
||||
noSuchKey *s3types.NoSuchKey
|
||||
)
|
||||
if errors.As(err, ¬Found) || errors.As(err, &noSuchKey) {
|
||||
if IsNotFound(err) {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
@@ -233,6 +229,18 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// IsNotFound reports whether err indicates that an object does not exist.
|
||||
// Head and Get requests surface a missing object as different SDK types,
|
||||
// so both are checked here.
|
||||
func IsNotFound(err error) bool {
|
||||
var (
|
||||
notFound *s3types.NotFound
|
||||
noSuchKey *s3types.NoSuchKey
|
||||
)
|
||||
|
||||
return errors.As(err, ¬Found) || errors.As(err, &noSuchKey)
|
||||
}
|
||||
|
||||
// ObjectInfo contains information about an S3 object.
|
||||
// It is used by ListObjectsStream to return object metadata
|
||||
// along with any errors encountered during listing.
|
||||
|
||||
+16
-1
@@ -38,14 +38,29 @@ func (s *S3Storer) PutWithProgress(
|
||||
}
|
||||
|
||||
// Get retrieves data from the specified key.
|
||||
// Returns ErrNotFound if the object does not exist.
|
||||
func (s *S3Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) {
|
||||
return s.client.GetObject(ctx, key)
|
||||
rc, err := s.client.GetObject(ctx, key)
|
||||
if err != nil {
|
||||
if s3.IsNotFound(err) {
|
||||
return nil, fmt.Errorf("get %q: %w", key, ErrNotFound)
|
||||
}
|
||||
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return rc, nil
|
||||
}
|
||||
|
||||
// Stat returns metadata about an object without retrieving its contents.
|
||||
// Returns ErrNotFound if the object does not exist.
|
||||
func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) {
|
||||
info, err := s.client.StatObject(ctx, key)
|
||||
if err != nil {
|
||||
if s3.IsNotFound(err) {
|
||||
return nil, fmt.Errorf("stat %q: %w", key, ErrNotFound)
|
||||
}
|
||||
|
||||
return nil, err
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
package storage_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/johannesboyne/gofakes3"
|
||||
"github.com/johannesboyne/gofakes3/backend/s3mem"
|
||||
|
||||
"sneak.berlin/go/vaultik/internal/s3"
|
||||
"sneak.berlin/go/vaultik/internal/storage"
|
||||
)
|
||||
|
||||
// TestS3StorerMissingKeyMapsToErrNotFound verifies that the s3 backend reports
|
||||
// a missing object as storage.ErrNotFound, matching the file and rclone
|
||||
// backends and the Storer contract. Without the mapping, Get and Stat leak the
|
||||
// raw SDK error and errors.Is(err, storage.ErrNotFound) is false.
|
||||
//
|
||||
//nolint:paralleltest // shares an in-process S3 server via t.Cleanup
|
||||
func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
|
||||
const bucket = "test-bucket"
|
||||
|
||||
backend := s3mem.New()
|
||||
|
||||
err := backend.CreateBucket(bucket)
|
||||
if err != nil {
|
||||
t.Fatalf("create bucket: %v", err)
|
||||
}
|
||||
|
||||
srv := httptest.NewServer(gofakes3.New(backend).Server())
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
client, err := s3.NewClient(ctx, s3.Config{
|
||||
Endpoint: srv.URL,
|
||||
Bucket: bucket,
|
||||
AccessKeyID: "test",
|
||||
SecretAccessKey: "test",
|
||||
Region: "us-east-1",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("new client: %v", err)
|
||||
}
|
||||
|
||||
storer := storage.NewS3Storer(client)
|
||||
|
||||
_, err = storer.Get(ctx, "does-not-exist")
|
||||
if !errors.Is(err, storage.ErrNotFound) {
|
||||
t.Errorf("Get on missing key: got %v, want ErrNotFound", err)
|
||||
}
|
||||
|
||||
_, err = storer.Stat(ctx, "does-not-exist")
|
||||
if !errors.Is(err, storage.ErrNotFound) {
|
||||
t.Errorf("Stat on missing key: got %v, want ErrNotFound", err)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user