Map s3 not-found to storage.ErrNotFound in Get and Stat (closes #129)
check / check (pull_request) Successful in 2m47s
check / check (pull_request) Successful in 2m47s
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
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