Skip to content

Expand checks for S3 200 errors to more operations - #3493

Open
Madrigal wants to merge 8 commits into
mainfrom
fix-s3-200-error
Open

Expand checks for S3 200 errors to more operations#3493
Madrigal wants to merge 8 commits into
mainfrom
fix-s3-200-error

Conversation

@Madrigal

Copy link
Copy Markdown
Contributor

Expand S3 200-with-error handling to all qualifying operations

Motivation

We have a customization to identify when s3 returns a response with a 200 status code but it actually contains an error. We in Go had it just for a handful of operation, but internal specification mentions that this can happen in way more places. Changed our codegen to expand on which operations we add this check.

Changes

Codegen (S3ErrorWith200Status.java): Replaced the hardcoded 3-operation
allowlist with the Smithy selector from the internal specification. The
middleware is now registered on all S3 operations whose output does NOT contain
an @httpPayload member targeting a @streaming shape or a string. This
matches the selector:

operation :not(:test(
    -[output]-> structure
    > member[trait|httpPayload]
    > :is([trait|streaming], string)))

Runtime (handle_200_error.go): Updated the empty-body case to pass through
instead of returning a deserialization error. Per the specification's "Response
parsing runtime safety" requirement, the SDK MUST NOT fail the request if parsing
fails — it must continue regular response handling. This was necessary because
many S3 operations (DELETE operations, WriteGetObjectResponse, etc.) can
legitimately return empty 200 responses.

Test (handle_200_error_test.go): Updated the 200NoResponse test case to
expect success on an empty body, matching the new behavior.

Backwards compatibility

This change may surface errors that were previously silently swallowed. For
operations beyond the original 3, a 200 response containing an <Error> XML
root element will now be treated as an error and retried (if retryable) rather
than being deserialized as a successful response. In practice:

  • InternalError / SlowDown: These will now be retried automatically on all
    S3 operations. Previously, only CopyObject, CompleteMultipartUpload, and
    UploadPartCopy retried them. This is a correctness improvement — customers
    were silently receiving error bodies treated as success on other operations.
  • Empty 200 responses: No longer cause a deserialization error. Operations
    that return empty bodies (e.g. DELETE, PUT with no response content) are
    unaffected.
  • Non-XML 200 responses: Operations with streaming or string payloads
    (GetObject, SelectObjectContent, GetBucketPolicy) are excluded by the
    codegen filter, so their behavior is unchanged.

The net effect is that customers who previously received silent data corruption
(error XML deserialized as a successful response) will now correctly receive an
error or benefit from automatic retry.

@Madrigal
Madrigal requested a review from a team July 24, 2026 21:57
@Madrigal
Madrigal marked this pull request as draft July 24, 2026 21:57
@Madrigal
Madrigal marked this pull request as ready for review July 30, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants