From c1c096e9a9ef269077ef6858de335611f9955023 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:44:01 +0800 Subject: [PATCH] fix(storage): map archived S3 objects to a clear sentinel; consolidate AWS error mapping Three awsObject read methods each mapped AWS S3 errors inline, and they disagreed: Size checked both NoSuchKey and NotFound, while WriteTo and OpenRangeReader checked only NoSuchKey. HeadObject (Size) surfaces NotFound but GetObject (WriteTo/OpenRangeReader) surfaces NoSuchKey, so a genuinely-missing object was reported as ErrObjectNotExist from one path and as a raw AWS error from another. In addition, an object lifecycle-transitioned to an archived storage class (GLACIER/DEEP_ARCHIVE) fails GetObject with InvalidObjectState, which no path mapped: a resume/create of such a template surfaced an opaque S3 error (a 500), with no signal that the fix is to rebuild the template. Consolidate the mapping into mapAWSError: NoSuchKey and NotFound -> the existing ErrObjectNotExist; InvalidObjectState -> a new ErrObjectArchived sentinel; anything else -> nil so the caller keeps its own operation-specific context (in particular OpenRangeReader keeps its "failed to create S3 range reader for %q" wrapper for unmapped errors). All three methods now go through it, fixing the NotFound inconsistency. The orchestrator Create handler maps ErrObjectArchived to FailedPrecondition with an actionable message ("archived and not directly accessible, please rebuild the template"), alongside the existing not-found branch; storage metrics gain an "archived" outcome. Rebased onto current main (methods now use errors.AsType, a Go 1.26 stdlib generic). Follows the review on the original #3147 by preserving per-operation error context on unmapped errors, and supersedes #3138 (the narrower InvalidObjectState-only change). Adds TestMapAWSError. Closes #3138. --- packages/orchestrator/pkg/server/sandboxes.go | 7 ++++ .../shared/pkg/storage/map_aws_error_test.go | 41 +++++++++++++++++++ packages/shared/pkg/storage/metrics.go | 5 +++ packages/shared/pkg/storage/storage.go | 5 +++ packages/shared/pkg/storage/storage_aws.go | 36 ++++++++++++---- 5 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 packages/shared/pkg/storage/map_aws_error_test.go diff --git a/packages/orchestrator/pkg/server/sandboxes.go b/packages/orchestrator/pkg/server/sandboxes.go index 6abe238390..42346ba652 100644 --- a/packages/orchestrator/pkg/server/sandboxes.go +++ b/packages/orchestrator/pkg/server/sandboxes.go @@ -430,6 +430,13 @@ func (s *Server) Create(ctx context.Context, req *orchestrator.SandboxCreateRequ return nil, status.Errorf(codes.FailedPrecondition, "sandbox files for '%s' not found", req.GetSandbox().GetSandboxId()) } + if errors.Is(err, storage.ErrObjectArchived) { + // The template's objects were lifecycle-transitioned to an archived + // storage class and cannot be read directly; the template must be rebuilt. + telemetry.ReportError(ctx, "sandbox files archived", err, telemetry.WithSandboxID(req.GetSandbox().GetSandboxId())) + + return nil, status.Errorf(codes.FailedPrecondition, "sandbox files for '%s' are archived and not directly accessible, please rebuild the template", req.GetSandbox().GetSandboxId()) + } if errors.Is(err, sandbox.ErrVcpuExceedsSnapshot) { logger.L().Warn(ctx, "resume refused: vcpu exceeds snapshot", zap.Error(err), zap.Int64("vcpu", vcpu), diff --git a/packages/shared/pkg/storage/map_aws_error_test.go b/packages/shared/pkg/storage/map_aws_error_test.go new file mode 100644 index 0000000000..d4de50e00f --- /dev/null +++ b/packages/shared/pkg/storage/map_aws_error_test.go @@ -0,0 +1,41 @@ +package storage + +import ( + "errors" + "fmt" + "testing" + + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/stretchr/testify/assert" +) + +// TestMapAWSError pins the AWS S3 error -> storage sentinel mapping shared by +// WriteTo, OpenRangeReader and Size: NoSuchKey (GetObject) and NotFound +// (HeadObject) both mean gone; InvalidObjectState means archived; anything else +// returns nil so the caller keeps its own operation-specific context. +func TestMapAWSError(t *testing.T) { + t.Parallel() + + for name, tc := range map[string]struct { + err error + want error + }{ + "NoSuchKey -> not exist": {&types.NoSuchKey{}, ErrObjectNotExist}, + "NotFound -> not exist": {&types.NotFound{}, ErrObjectNotExist}, + "InvalidObjectState -> archived": {&types.InvalidObjectState{}, ErrObjectArchived}, + "wrapped NoSuchKey -> not exist": {fmt.Errorf("get: %w", &types.NoSuchKey{}), ErrObjectNotExist}, + "wrapped archived -> archived": {fmt.Errorf("get: %w", &types.InvalidObjectState{}), ErrObjectArchived}, + "unknown error -> nil (keep ctx)": {errors.New("some transient network error"), nil}, + "nil -> nil": {nil, nil}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + got := mapAWSError(tc.err) + if tc.want == nil { + assert.Nil(t, got) + return + } + assert.ErrorIs(t, got, tc.want) + }) + } +} diff --git a/packages/shared/pkg/storage/metrics.go b/packages/shared/pkg/storage/metrics.go index 2b3a0da751..198329e73d 100644 --- a/packages/shared/pkg/storage/metrics.go +++ b/packages/shared/pkg/storage/metrics.go @@ -38,6 +38,9 @@ const ( OutcomeErrIO = "err_io" OutcomeErrTimeout = "err_timeout" OutcomeTransitioned = "transitioned" + // OutcomeArchived: the object exists but is in an archived storage class + // that does not allow direct reads. + OutcomeArchived = "archived" // OutcomeContended is a writeback skipped because another goroutine held the // NFS chunk lock — normal cache dedup, nothing written. OutcomeContended = "contended" @@ -82,6 +85,8 @@ func Outcome(err error) string { return OutcomeOK case errors.Is(err, ErrObjectNotExist), errors.Is(err, fs.ErrNotExist): return OutcomeNotFound + case errors.Is(err, ErrObjectArchived): + return OutcomeArchived case errors.Is(err, context.Canceled): return OutcomeErrCanceled case errors.Is(err, context.DeadlineExceeded): diff --git a/packages/shared/pkg/storage/storage.go b/packages/shared/pkg/storage/storage.go index f517dfdede..f490829ecb 100644 --- a/packages/shared/pkg/storage/storage.go +++ b/packages/shared/pkg/storage/storage.go @@ -21,6 +21,11 @@ var tracer = otel.Tracer("github.com/e2b-dev/infra/packages/shared/pkg/storage") var ErrObjectNotExist = errors.New("object does not exist") +// ErrObjectArchived means the object exists but is in an archived storage class +// (e.g. GLACIER, DEEP_ARCHIVE) that does not allow direct reads. It must be +// restored or the template rebuilt before the object can be used. +var ErrObjectArchived = errors.New("object is archived and not directly accessible") + // ErrSignedUploadURLUnsupported means the provider cannot sign an upload URL usable by an // external client. Callers that only need the URL on a cache miss may tolerate it. var ErrSignedUploadURLUnsupported = errors.New("signed upload URLs are not supported by this storage provider") diff --git a/packages/shared/pkg/storage/storage_aws.go b/packages/shared/pkg/storage/storage_aws.go index 52c8d49ab1..4aeb99e3de 100644 --- a/packages/shared/pkg/storage/storage_aws.go +++ b/packages/shared/pkg/storage/storage_aws.go @@ -249,6 +249,28 @@ func (s *awsStorage) OpenBlob(_ context.Context, path string) (Blob, error) { }, nil } +// mapAWSError maps a well-known S3 error to a storage-layer sentinel, or nil if +// err is not one the storage layer recognizes. NoSuchKey (GetObject) and +// NotFound (HeadObject) both mean the object is gone; InvalidObjectState means +// it is in an archived storage class. Callers that get nil keep their own +// operation-specific error context. +func mapAWSError(err error) error { + switch { + case isAWSErr[*types.NoSuchKey](err), isAWSErr[*types.NotFound](err): + return ErrObjectNotExist + case isAWSErr[*types.InvalidObjectState](err): + return ErrObjectArchived + default: + return nil + } +} + +func isAWSErr[T error](err error) bool { + _, ok := errors.AsType[T](err) + + return ok +} + func (o *awsObject) WriteTo(ctx context.Context, dst io.Writer) (n int64, err error) { start := time.Now() defer func() { RecordReadBlob(ctx, time.Since(start), n, o.path, SourceAWS, err) }() @@ -258,8 +280,8 @@ func (o *awsObject) WriteTo(ctx context.Context, dst io.Writer) (n int64, err er resp, err := o.client.GetObject(ctx, &s3.GetObjectInput{Bucket: &o.bucketName, Key: &o.path}) if err != nil { - if _, ok := errors.AsType[*types.NoSuchKey](err); ok { - return 0, ErrObjectNotExist + if mapped := mapAWSError(err); mapped != nil { + return 0, mapped } return 0, err @@ -402,8 +424,8 @@ func (o *awsObject) openRangeReader(ctx context.Context, off, length int64) (Ran Range: aws.String(fmt.Sprintf("bytes=%d-%d", off, off+length-1)), }) if err != nil { - if _, ok := errors.AsType[*types.NoSuchKey](err); ok { - return nil, ErrObjectNotExist + if mapped := mapAWSError(err); mapped != nil { + return nil, mapped } return nil, fmt.Errorf("failed to create S3 range reader for %q: %w", o.path, err) @@ -422,10 +444,8 @@ func (o *awsObject) Size(ctx context.Context) (_ int64, err error) { resp, err := o.client.HeadObject(ctx, &s3.HeadObjectInput{Bucket: &o.bucketName, Key: &o.path}) if err != nil { - var nsk *types.NoSuchKey - var nfd *types.NotFound - if errors.As(err, &nsk) || errors.As(err, &nfd) { - return 0, ErrObjectNotExist + if mapped := mapAWSError(err); mapped != nil { + return 0, mapped } return 0, err