Skip to content

fix(smithy-aws-core): omit Result wrapper for empty awsQuery output#714

Merged
Alan4506 merged 3 commits into
smithy-lang:developfrom
Alan4506:fix/awsquery-empty-output
Jun 10, 2026
Merged

fix(smithy-aws-core): omit Result wrapper for empty awsQuery output#714
Alan4506 merged 3 commits into
smithy-lang:developfrom
Alan4506:fix/awsquery-empty-output

Conversation

@Alan4506

@Alan4506 Alan4506 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Problem:

The awsQuery protocol returns XML responses wrapped in two layers: an outer <{OperationName}Response> element, and inside it an <{OperationName}Result> element and a <ResponseMetadata> element. For example, Amazon SNS CreateTopic returns:

<CreateTopicResponse xmlns="http://sns.amazonaws.com/doc/2010-03-31/">
  <CreateTopicResult>
    <TopicArn>arn:aws:sns:us-east-1:111122223333:my-topic</TopicArn>
  </CreateTopicResult>
  <ResponseMetadata>
    <RequestId>...</RequestId>
  </ResponseMetadata>
</CreateTopicResponse>

To deserialize this, AwsQueryClientProtocol._response_wrapper_elements tells the XML deserializer to descend through both wrapper elements before reading members. The current implementation always returns both:

def _response_wrapper_elements(
    self,
    operation: APIOperation[SerializeableShape, DeserializeableShape],
) -> tuple[str, str]:
    return (
        f"{operation.schema.id.name}Response",
        f"{operation.schema.id.name}Result",
    )

The problem is that some operations' output shapes do not have members, so they do not get a <{OperationName}Result> element from the service. The response body contains only <ResponseMetadata> inside the outer wrapper. Requiring <{OperationName}Result> then fails.

This was found while testing Amazon SNS SetTopicAttributes and DeleteTopic operations. For example:

<SetTopicAttributesResponse xmlns="http://sns.amazonaws.com/doc/2010-03-31/">
  <ResponseMetadata>
    <RequestId>...</RequestId>
  </ResponseMetadata>
</SetTopicAttributesResponse>

The call failed with:

XMLParseError: Error parsing XML: Expected element 'SetTopicAttributesResult', got 'ResponseMetadata'

Description of changes:

In _response_wrapper_elements, only include the <{OperationName}Result> wrapper when the operation's output shape has members. Operations with empty output are unwrapped using just the outer <{OperationName}Response>. The return type is changed from tuple[str, str] to tuple[str, ...] since it now returns either one or two wrapper names.

Testing:

  • Added a unit test in tests/unit/aio/test_protocols.py that deserializes an empty-output awsQuery response, and asserts it returns the output shape instead of raising.
  • Generated Amazon SNS client locally, and verified its operation worked well: CreateTopic / GetTopicAttributes / SetTopicAttributes / ListTopics / DeleteTopic / Subscribe.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Alan4506 Alan4506 requested a review from a team as a code owner June 8, 2026 23:02
@Alan4506 Alan4506 changed the title Fix/awsquery empty output fix(smithy-aws-core): omit Result wrapper for empty awsQuery output Jun 8, 2026
@arandito

Copy link
Copy Markdown
Contributor

Thanks @Alan4506! The fix looks good and makes sense. My only concern is dropping the added unit test and instead using the newly added Smithy protocol test for this exact case: smithy-lang/smithy#3106. The protocol tests should be the source of truth for protocol behavior. Can you confirm the new test is passing?

We have a PR open to upgrade us to smithy 1.71.0 that is blocked because this new test is failing: #703. This fix should unblock that as well.

@Alan4506

Copy link
Copy Markdown
Contributor Author

Thanks @Alan4506! The fix looks good and makes sense. My only concern is dropping the added unit test and instead using the newly added Smithy protocol test for this exact case: smithy-lang/smithy#3106. The protocol tests should be the source of truth for protocol behavior. Can you confirm the new test is passing?

We have a PR open to upgrade us to smithy 1.71.0 that is blocked because this new test is failing: #703. This fix should unblock that as well.

Thanks @arandito! I've removed the unit test and verfied that the newly added Smithy protocol test:

  • passed with our fix.
  • failed without our fix.

@arandito arandito left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@Alan4506 Alan4506 merged commit b28af3c into smithy-lang:develop Jun 10, 2026
8 checks passed
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