Skip to content

fix(storage): map archived S3 objects to a clear sentinel; consolidate AWS error mapping (rebase of #3147) - #3734

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/aws-archived-object-mapping
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/aws-archived-object-mapping

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Rebased follow-up consolidating #3147 and #3138 (both closed) against current main. Two real issues, both still present.

1. Inconsistent "object missing" mapping

The three awsObject read methods each mapped S3 errors inline and disagreed: Size (HeadObject) checked both NoSuchKey and NotFound, while WriteTo/OpenRangeReader (GetObject) checked only NoSuchKey. So a genuinely-missing object was ErrObjectNotExist from one path and a raw AWS error from another.

2. Archived objects surface an opaque error

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 hint that the fix is to rebuild the template.

Fix

Consolidate into mapAWSError:

  • NoSuchKey, NotFound → existing ErrObjectNotExist
  • InvalidObjectState → new ErrObjectArchived
  • anything else → nil, so the caller keeps its own operation-specific context

All three methods now go through it (fixing the NotFound inconsistency), and OpenRangeReader keeps its "failed to create S3 range reader for %q" wrapper for unmapped errors — per the review on #3147.

The orchestrator Create handler maps ErrObjectArchived to FailedPrecondition with an actionable message ("archived and not directly accessible, please rebuild the template"), beside the existing not-found branch; storage metrics gain an archived outcome.

Notes

Closes #3138.

…e 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 e2b-dev#3147 by preserving per-operation
error context on unmapped errors, and supersedes e2b-dev#3138 (the narrower
InvalidObjectState-only change). Adds TestMapAWSError.

Closes e2b-dev#3138.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants