From 0a405316da05a60561349997169f49fd60fe07d0 Mon Sep 17 00:00:00 2001 From: ubaskota <19787410+ubaskota@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:47:00 -0400 Subject: [PATCH] Attach response metadata to operation outputs and errors, escaping collisions --- .../smithy/python/codegen/CodegenUtils.java | 12 ++ .../python/codegen/PythonSymbolProvider.java | 31 +++++ .../smithy/python/codegen/RuntimeTypes.java | 6 + .../generators/StructureGenerator.java | 35 ++++++ .../codegen/PythonSymbolProviderTest.java | 70 +++++++++++ ...ture-70d7c47f420d4c4b8e5bfca35eb43d1e.json | 4 + .../_private/query/__init__.py | 2 + .../smithy_aws_core/_private/query/_xml.py | 20 +++ .../smithy_aws_core/_private/query/errors.py | 24 +--- .../_private/query/metadata.py | 26 ++++ .../src/smithy_aws_core/aio/protocols.py | 75 ++++++++++- .../src/smithy_aws_core/utils.py | 48 +++++++ .../tests/unit/aio/test_protocols.py | 119 ++++++++++++++++++ .../smithy-aws-core/tests/unit/test_query.py | 36 ++++++ .../smithy-aws-core/tests/unit/test_utils.py | 93 ++++++++++++++ ...king-ffcfa27938c44f9f9b839d62cde984bc.json | 4 + ...ture-b5b41df5a72d4340a396d624302bbbac.json | 4 + .../smithy-core/src/smithy_core/aio/client.py | 47 ++++++- .../smithy_core/aio/interfaces/__init__.py | 23 ++++ .../smithy-core/src/smithy_core/exceptions.py | 14 +++ .../smithy-core/src/smithy_core/response.py | 47 +++++++ .../tests/unit/aio/_pipeline_harness.py | 66 ++++++++++ .../smithy-core/tests/unit/aio/test_client.py | 74 ++++++++++- ...ture-21a43ab4b58f4e2d90e0e3f59503f370.json | 4 + .../src/smithy_http/aio/protocols.py | 14 +++ .../tests/unit/aio/test_protocols.py | 23 +++- 26 files changed, 892 insertions(+), 29 deletions(-) create mode 100644 packages/smithy-aws-core/.changes/next-release/smithy-aws-core-feature-70d7c47f420d4c4b8e5bfca35eb43d1e.json create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py create mode 100644 packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py create mode 100644 packages/smithy-core/.changes/next-release/smithy-core-breaking-ffcfa27938c44f9f9b839d62cde984bc.json create mode 100644 packages/smithy-core/.changes/next-release/smithy-core-feature-b5b41df5a72d4340a396d624302bbbac.json create mode 100644 packages/smithy-core/src/smithy_core/response.py create mode 100644 packages/smithy-http/.changes/next-release/smithy-http-feature-21a43ab4b58f4e2d90e0e3f59503f370.json diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java index e1f6b5f98..cf1c66444 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/CodegenUtils.java @@ -55,6 +55,18 @@ public final class CodegenUtils { */ public static final int MAX_PREFERRED_LINE_LENGTH = 88; + /** + * The name of the attribute carrying response metadata on operation outputs + * and modeled errors. + * + *

Declared on {@code CallError} for errors and emitted onto operation + * outputs by the structure generator. Because a service is free to model a + * member of the same name, the name is reserved on the members of those + * shapes and escaped if it collides. Both the emitter and the escaper read it + * from here so that they cannot disagree. + */ + public static final String RESPONSE_METADATA_MEMBER = "response_metadata"; + static final Set ERROR_MESSAGE_MEMBER_NAMES = SetUtils.of( "errormessage", "error_message", diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java index 0a946f441..22202c88a 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/PythonSymbolProvider.java @@ -16,6 +16,7 @@ import software.amazon.smithy.codegen.core.SymbolProvider; import software.amazon.smithy.codegen.core.SymbolReference; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.OperationIndex; import software.amazon.smithy.model.loader.Prelude; import software.amazon.smithy.model.shapes.BigDecimalShape; import software.amazon.smithy.model.shapes.BigIntegerShape; @@ -74,6 +75,7 @@ public final class PythonSymbolProvider implements SymbolProvider, ShapeVisitor< private final PythonSettings settings; private final ServiceShape service; private final Set allShapeNames; + private final OperationIndex operationIndex; public PythonSymbolProvider(Model model, PythonSettings settings) { this.model = model; @@ -107,6 +109,9 @@ public PythonSymbolProvider(Model model, PythonSettings settings) { // Collect all shape names that will be generated as PascalCase classes in models.py. // Used to detect collisions with synthesized names (union variants, unknown types). this.allShapeNames = collectAllShapeNames(); + + // Built once because toMemberName runs for every member of every shape. + this.operationIndex = OperationIndex.of(model); } /** @@ -149,12 +154,38 @@ public String toMemberName(MemberShape shape) { } var container = model.expectShape(shape.getContainer()); + + // The response metadata attribute is added to operation outputs and errors + // by the generator, so a modeled member of the same name is escaped to keep + // it from shadowing the attribute. + if (CodegenUtils.RESPONSE_METADATA_MEMBER.equals(memberName) + && carriesResponseMetadata(container)) { + memberName = escapeWord(memberName); + LOGGER.warning(() -> format( + "Renamed member %s to \"%s\" because \"%s\" is reserved for response metadata.", + shape.getId(), + escapeWord(CodegenUtils.RESPONSE_METADATA_MEMBER), + CodegenUtils.RESPONSE_METADATA_MEMBER)); + } + if (container.isEnumShape() || container.isIntEnumShape()) { memberName = memberName.toUpperCase(Locale.ENGLISH); } return memberName; } + /** + * Whether a shape is given a response metadata attribute, and so has a member + * name that must be kept clear. + * + *

Operation inputs and other structures are not given the attribute, so + * their members are left alone. + */ + private boolean carriesResponseMetadata(Shape container) { + return container.hasTrait(ErrorTrait.class) + || operationIndex.isOutputStructure(container); + } + private String getDefaultShapeName(Shape shape) { // Use the service-aliased name return StringUtils.capitalize(shape.getId().getName(service)); diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RuntimeTypes.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RuntimeTypes.java index 7c5aed8e5..059faa126 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RuntimeTypes.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/RuntimeTypes.java @@ -40,6 +40,12 @@ public final class RuntimeTypes { public static final Symbol TYPE_REGISTRY = createSymbol("documents", "TypeRegistry", SmithyPythonDependency.SMITHY_CORE); + // smithy_core.response + public static final Symbol RESPONSE_METADATA = + createSymbol("response", "ResponseMetadata", SmithyPythonDependency.SMITHY_CORE); + public static final Symbol EMPTY_RESPONSE_METADATA = + createSymbol("response", "EMPTY_RESPONSE_METADATA", SmithyPythonDependency.SMITHY_CORE); + // smithy_core.exceptions public static final Symbol MODELED_ERROR = createSymbol("exceptions", "ModeledError", SmithyPythonDependency.SMITHY_CORE); diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java index 375dab7bd..ae43aa224 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java @@ -19,6 +19,7 @@ import software.amazon.smithy.codegen.core.SymbolProvider; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.NullableIndex; +import software.amazon.smithy.model.knowledge.OperationIndex; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.Shape; @@ -104,6 +105,7 @@ private void renderStructure() { class $L: ${C|} + ${C|} ${C|} ${C|} @@ -116,6 +118,7 @@ class $L: symbol.getName(), writer.consumer(w -> writeClassDocs()), writer.consumer(w -> writeProperties()), + writer.consumer(w -> writeResponseMetadataProperty()), writer.consumer(w -> generateSerializeMethod()), writer.consumer(w -> generateDeserializeMethod()), writer.consumer(w -> generateSmithyDefaultMethod())); @@ -177,6 +180,38 @@ private void writeClassDocs() { writer.writeDocs(docs, context); } + /** + * Writes the response metadata attribute onto operation outputs. + * + *

The attribute is not part of the service's modeled data, so it is excluded + * from equality and from the generated repr. Excluding it from equality also + * keeps generated protocol tests comparing shapes by their modeled members + * alone. + * + *

Errors receive the same attribute by inheriting it from the service error + * base class rather than having it written here. + */ + private void writeResponseMetadataProperty() { + if (!OperationIndex.of(model).isOutputStructure(shape)) { + return; + } + + writer.addStdlibImport("dataclasses", "field"); + writer.write(""" + $L: $T = field(default=$T, repr=False, compare=False) + $C + """, + CodegenUtils.RESPONSE_METADATA_MEMBER, + RuntimeTypes.RESPONSE_METADATA, + RuntimeTypes.EMPTY_RESPONSE_METADATA, + writer.consumer(w -> w.writeDocs(""" + Metadata about the response that produced this output. + + Use this to recover the request identifiers a service's support team \ + needs in order to investigate a call. Members of the metadata are \ + individually optional.""", context))); + } + private void writeProperties() { for (MemberShape member : requiredMembers) { writer.pushState(); diff --git a/codegen/core/src/test/java/software/amazon/smithy/python/codegen/PythonSymbolProviderTest.java b/codegen/core/src/test/java/software/amazon/smithy/python/codegen/PythonSymbolProviderTest.java index dcf1f46e2..0102a560c 100644 --- a/codegen/core/src/test/java/software/amazon/smithy/python/codegen/PythonSymbolProviderTest.java +++ b/codegen/core/src/test/java/software/amazon/smithy/python/codegen/PythonSymbolProviderTest.java @@ -108,6 +108,76 @@ public void testOperationNameCollidingWithClientMethodIsEscaped() { .getName()); } + @Test + public void testResponseMetadataIsEscapedOnOutputsAndErrors() { + Model model = loadModel(RESPONSE_METADATA_MODEL); + PythonSymbolProvider provider = createProvider(model); + + assertEquals("response_metadata_", memberName(provider, model, "GetThingOutput$responseMetadata")); + assertEquals("response_metadata_", memberName(provider, model, "ThingError$responseMetadata")); + } + + @Test + public void testResponseMetadataIsNotEscapedOnOtherShapes() { + // Only outputs and errors are given the attribute, so members elsewhere + // must keep their natural name. + Model model = loadModel(RESPONSE_METADATA_MODEL); + PythonSymbolProvider provider = createProvider(model); + + assertEquals("response_metadata", memberName(provider, model, "GetThingInput$responseMetadata")); + assertEquals("response_metadata", memberName(provider, model, "Nested$responseMetadata")); + } + + @Test + public void testUnrelatedMembersOnOutputsAndErrorsAreUnaffected() { + Model model = loadModel(RESPONSE_METADATA_MODEL); + PythonSymbolProvider provider = createProvider(model); + + assertEquals("thing_arn", memberName(provider, model, "GetThingOutput$thingArn")); + assertEquals("retry_after", memberName(provider, model, "ThingError$retryAfter")); + } + + private static final String RESPONSE_METADATA_MODEL = """ + $version: "2" + namespace smithy.example + + service TestService { + version: "2024-01-01" + operations: [GetThing] + errors: [ThingError] + } + + operation GetThing { + input: GetThingInput + output: GetThingOutput + } + + structure GetThingInput { + responseMetadata: String + } + + structure GetThingOutput { + responseMetadata: String + thingArn: String + nested: Nested + } + + structure Nested { + responseMetadata: String + } + + @error("client") + structure ThingError { + responseMetadata: String + retryAfter: String + } + """; + + private static String memberName(PythonSymbolProvider provider, Model model, String relativeId) { + var member = model.expectShape(ShapeId.from(NS + "#" + relativeId), MemberShape.class); + return provider.toMemberName(member); + } + private static Model loadModel(String smithyIdl) { return Model.assembler().addUnparsedModel("test.smithy", smithyIdl).assemble().unwrap(); } diff --git a/packages/smithy-aws-core/.changes/next-release/smithy-aws-core-feature-70d7c47f420d4c4b8e5bfca35eb43d1e.json b/packages/smithy-aws-core/.changes/next-release/smithy-aws-core-feature-70d7c47f420d4c4b8e5bfca35eb43d1e.json new file mode 100644 index 000000000..a1c39565a --- /dev/null +++ b/packages/smithy-aws-core/.changes/next-release/smithy-aws-core-feature-70d7c47f420d4c4b8e5bfca35eb43d1e.json @@ -0,0 +1,4 @@ +{ + "type": "feature", + "description": "AWS protocols now report request identifiers on operation outputs and errors, read from the `x-amzn-requestid`, `x-amz-request-id`, and `x-amz-id-2` headers. awsQuery reads them from the response body." +} diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/__init__.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/__init__.py index 694350d1f..9fee18bb5 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/__init__.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/__init__.py @@ -2,9 +2,11 @@ # SPDX-License-Identifier: Apache-2.0 from .errors import create_aws_query_error +from .metadata import parse_aws_query_request_id from .serializers import QueryShapeSerializer __all__ = ( "QueryShapeSerializer", "create_aws_query_error", + "parse_aws_query_request_id", ) diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py new file mode 100644 index 000000000..eec6a77a1 --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/_xml.py @@ -0,0 +1,20 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Helpers for reading awsQuery response bodies that are not shape-modeled.""" + +from xml.etree.ElementTree import Element + + +def local_name(tag: str) -> str: + """Strip namespace URI from an element tag: {uri}local -> local.""" + if tag.startswith("{"): + return tag.split("}", 1)[1] + return tag + + +def find_child(element: Element, name: str) -> Element | None: + """Return the first child element whose local name matches ``name``.""" + for child in element: + if local_name(child.tag) == name: + return child + return None diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py index af86fab12..f3726af15 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/errors.py @@ -1,7 +1,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 from typing import TYPE_CHECKING, Any -from xml.etree.ElementTree import Element, ParseError, fromstring +from xml.etree.ElementTree import ParseError, fromstring from smithy_core.documents import TypeRegistry from smithy_core.exceptions import ( @@ -15,6 +15,7 @@ from smithy_core.shapes import ShapeID from ...traits import AwsQueryErrorTrait +from ._xml import find_child, local_name try: from smithy_xml import XMLCodec @@ -34,21 +35,6 @@ def _assert_xml() -> None: ) -def _local_name(tag: str) -> str: - """Strip namespace URI from an element tag: {uri}local -> local.""" - if tag.startswith("{"): - return tag.split("}", 1)[1] - return tag - - -def _find_child(element: Element, name: str) -> Element | None: - """Return the first child element whose local name matches ``name``.""" - for child in element: - if _local_name(child.tag) == name: - return child - return None - - def _parse_aws_query_error_code( body: bytes, wrapper_elements: tuple[str, ...] ) -> str | None: @@ -59,15 +45,15 @@ def _parse_aws_query_error_code( return None if wrapper_elements: - if _local_name(element.tag) != wrapper_elements[0]: + if local_name(element.tag) != wrapper_elements[0]: return None for wrapper in wrapper_elements[1:]: - next_element = _find_child(element, wrapper) + next_element = find_child(element, wrapper) if next_element is None: return None element = next_element - code_element = _find_child(element, "Code") + code_element = find_child(element, "Code") return code_element.text if code_element is not None else None diff --git a/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py new file mode 100644 index 000000000..c95b36d2a --- /dev/null +++ b/packages/smithy-aws-core/src/smithy_aws_core/_private/query/metadata.py @@ -0,0 +1,26 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from xml.etree.ElementTree import ParseError, fromstring + +from ._xml import find_child + + +def parse_aws_query_request_id(body: bytes) -> str | None: + """Parse the request ID from an awsQuery response body. + + There is no request ID header. Successes nest it under ``ResponseMetadata``, + errors put it directly under the root. + """ + try: + root = fromstring(body) # noqa: S314 + except ParseError: + return None + + metadata = find_child(root, "ResponseMetadata") + if metadata is not None: + nested = find_child(metadata, "RequestId") + if nested is not None and nested.text: + return nested.text + + direct = find_child(root, "RequestId") + return direct.text or None if direct is not None else None diff --git a/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py b/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py index a9f85f278..d1a54724f 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/aio/protocols.py @@ -1,6 +1,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 from collections.abc import Callable +from dataclasses import replace from inspect import iscoroutinefunction from io import BytesIO from typing import TYPE_CHECKING, Any, ClassVar, Final @@ -23,10 +24,11 @@ ) from smithy_core.interfaces import TypedProperties, URI from smithy_core.prelude import DOCUMENT +from smithy_core.response import ResponseMetadata from smithy_core.schemas import APIOperation, Schema from smithy_core.serializers import SerializeableShape from smithy_core.shapes import ShapeID, ShapeType -from smithy_core.types import TimestampFormat +from smithy_core.types import PropertyKey, TimestampFormat from smithy_http import tuples_to_fields from smithy_http.aio import HTTPRequest as _HTTPRequest from smithy_http.aio.interfaces import HTTPErrorIdentifier, HTTPRequest, HTTPResponse @@ -37,9 +39,15 @@ from smithy_http.deserializers import HTTPResponseDeserializer from .._private.query.errors import create_aws_query_error +from .._private.query.metadata import parse_aws_query_request_id from .._private.query.serializers import QueryShapeSerializer from ..traits import AwsJson1_0Trait, AwsJson1_1Trait, AwsQueryTrait, RestJson1Trait -from ..utils import parse_document_discriminator, parse_error_code, parse_retry_after +from ..utils import ( + parse_document_discriminator, + parse_error_code, + parse_response_metadata, + parse_retry_after, +) try: from smithy_json import JSONCodec, JSONDocument @@ -139,7 +147,24 @@ class AWSJSONDocument: # type: ignore[no-redef] pass -class RestJsonClientProtocol(HttpBindingClientProtocol): +class _AWSResponseMetadataMixin: + """Adds AWS request identifiers to extracted response metadata. + + Mixed into each AWS protocol ahead of its HTTP base class, which supplies + only the status code. AWS protocols do not share a common base, so this is + applied per protocol. + """ + + def extract_response_metadata( + self, + *, + response: HTTPResponse, + context: TypedProperties, + ) -> ResponseMetadata: + return parse_response_metadata(response) + + +class RestJsonClientProtocol(_AWSResponseMetadataMixin, HttpBindingClientProtocol): """An implementation of the aws.protocols#restJson1 protocol.""" _id: Final = RestJson1Trait.id @@ -252,7 +277,7 @@ def create_event_receiver[ ) -class _AWSJSONClientProtocol(HttpClientProtocol): +class _AWSJSONClientProtocol(_AWSResponseMetadataMixin, HttpClientProtocol): _error_identifier: Final = AWSErrorIdentifier() _id: ClassVar[ShapeID] @@ -451,7 +476,15 @@ class AwsJson11ClientProtocol(_AWSJSONClientProtocol): _content_type: ClassVar[str] = "application/x-amz-json-1.1" -class AwsQueryClientProtocol(HttpClientProtocol): +_QUERY_REQUEST_ID = PropertyKey(key="aws_query_request_id", value_type=str) +"""Where :py:class:`AwsQueryClientProtocol` records a body-sourced request ID. + +The body is only available while deserializing, so the value is stored there for +``extract_response_metadata`` to read back. +""" + + +class AwsQueryClientProtocol(_AWSResponseMetadataMixin, HttpClientProtocol): """An implementation of the aws.protocols#awsQuery protocol.""" _id: Final = AwsQueryTrait.id @@ -475,6 +508,28 @@ def payload_codec(self) -> "XMLCodec": def content_type(self) -> str: return self._content_type + def extract_response_metadata( + self, + *, + response: HTTPResponse, + context: TypedProperties, + ) -> ResponseMetadata: + """Report the request ID, using the one recorded from the body as a fallback. + + awsQuery normally carries the identifier in the body rather than a header, so + the mixin's header lookup usually finds nothing and the body value recorded + during deserialization is used. A header still wins when a service sends one. + """ + metadata = _AWSResponseMetadataMixin.extract_response_metadata( + self, response=response, context=context + ) + if metadata.request_id is not None: + return metadata + request_id = context.get(_QUERY_REQUEST_ID) + if request_id is None: + return metadata + return replace(metadata, request_id=request_id) + def serialize_request[ OperationInput: SerializeableShape, OperationOutput: DeserializeableShape, @@ -525,6 +580,16 @@ async def deserialize_response[ ) -> OperationOutput: body = await response.consume_body_async() + # Recorded before any branch below returns or raises, so successes, empty + # outputs and errors alike can report the identifier. Retry attempts share + # one properties object, so a body without an ID must clear any value a + # previous attempt left behind rather than let it be reported as this one's. + request_id = parse_aws_query_request_id(body) + if request_id is not None: + context[_QUERY_REQUEST_ID] = request_id + else: + context.pop(_QUERY_REQUEST_ID, None) + if not self._is_success(operation, context, response): raise await self._create_error( operation=operation, diff --git a/packages/smithy-aws-core/src/smithy_aws_core/utils.py b/packages/smithy-aws-core/src/smithy_aws_core/utils.py index 6e03b2fac..85c764643 100644 --- a/packages/smithy-aws-core/src/smithy_aws_core/utils.py +++ b/packages/smithy-aws-core/src/smithy_aws_core/utils.py @@ -3,13 +3,24 @@ import logging from smithy_core.documents import Document +from smithy_core.response import ResponseMetadata from smithy_core.shapes import ShapeID, ShapeType from smithy_http.aio.interfaces import HTTPResponse +from smithy_http.interfaces import Field _LOGGER = logging.getLogger(__name__) _RETRY_AFTER_HEADER = "x-amz-retry-after" +_REQUEST_ID_HEADERS = ("x-amzn-requestid", "x-amz-request-id") +"""Headers that may carry the request ID, in order of preference. + +Most services send ``x-amzn-requestid``. Services in the Amazon S3 lineage send +``x-amz-request-id`` instead. +""" + +_EXTENDED_REQUEST_ID_HEADER = "x-amz-id-2" + def parse_retry_after(response: HTTPResponse) -> float | None: """Parse the ``x-amz-retry-after`` header into a backoff duration in seconds. @@ -35,6 +46,43 @@ def parse_retry_after(response: HTTPResponse) -> float | None: return None +def _first_value(field: Field) -> str | None: + """The field's first value, or None if it has none or the first is empty. + + Identifiers are single opaque tokens, so a repeated header is read as its + first value rather than joined the way ``as_string`` would. + """ + return field.values[0] or None if field.values else None + + +def parse_response_metadata(response: HTTPResponse) -> ResponseMetadata: + """Extract AWS response metadata from an HTTP response. + + The request ID is read from the first present header in + ``_REQUEST_ID_HEADERS``. The extended request ID comes from + ``x-amz-id-2`` and is only sent by some services. + + Absent or empty headers are left unset rather than raising, since this + information is diagnostic and must never fail a call. + """ + request_id = None + for header in _REQUEST_ID_HEADERS: + if header in response.fields: + request_id = _first_value(response.fields[header]) + if request_id is not None: + break + + extended_request_id = None + if _EXTENDED_REQUEST_ID_HEADER in response.fields: + extended_request_id = _first_value(response.fields[_EXTENDED_REQUEST_ID_HEADER]) + + return ResponseMetadata( + request_id=request_id, + extended_request_id=extended_request_id, + http_status_code=response.status, + ) + + def parse_document_discriminator( document: Document, default_namespace: str | None ) -> ShapeID | None: diff --git a/packages/smithy-aws-core/tests/unit/aio/test_protocols.py b/packages/smithy-aws-core/tests/unit/aio/test_protocols.py index 1a1a31467..cfeeedf08 100644 --- a/packages/smithy-aws-core/tests/unit/aio/test_protocols.py +++ b/packages/smithy-aws-core/tests/unit/aio/test_protocols.py @@ -574,3 +574,122 @@ async def test_aws_query_returns_generic_error_for_unknown_code() -> None: "Unknown error for operation com.test#FailingOperation" " - status: 500, code: UnknownThing" ) + + +async def test_aws_query_reports_request_id_from_the_response_body() -> None: + protocol = AwsQueryClientProtocol(_SERVICE_SCHEMA, "2020-01-08") + context = TypedProperties() + response = HTTPResponse( + status=400, + fields=tuples_to_fields([]), + body=( + b"InvalidAction" + b"bad request" + b"body-request-id" + ), + ) + with pytest.raises(_ModeledQueryError): + await protocol.deserialize_response( + operation=_mock_operation( + _operation_schema("FailingOperation"), + error_schemas=[_INVALID_ACTION_ERROR_SCHEMA], + ), + request=cast(HTTPRequest, Mock()), + response=response, + error_registry=TypeRegistry( + {ShapeID("com.test#InvalidActionError"): _ModeledQueryError} + ), + context=context, + ) + + metadata = protocol.extract_response_metadata(response=response, context=context) + assert metadata.request_id == "body-request-id" + assert metadata.http_status_code == 400 + + +async def test_aws_query_prefers_a_request_id_header_when_one_is_sent() -> None: + protocol = AwsQueryClientProtocol(_SERVICE_SCHEMA, "2020-01-08") + context = TypedProperties() + response = HTTPResponse( + status=400, + fields=tuples_to_fields([("x-amzn-requestid", "header-request-id")]), + body=( + b"InvalidAction" + b"bad request" + b"body-request-id" + ), + ) + await _deserialize_query_error(protocol, response, context) + + metadata = protocol.extract_response_metadata(response=response, context=context) + assert metadata.request_id == "header-request-id" + + +async def test_aws_query_reports_no_request_id_when_the_body_has_none() -> None: + protocol = AwsQueryClientProtocol(_SERVICE_SCHEMA, "2020-01-08") + response = HTTPResponse( + status=200, fields=tuples_to_fields([]), body=b"" + ) + metadata = protocol.extract_response_metadata( + response=response, context=TypedProperties() + ) + assert metadata.request_id is None + assert metadata.http_status_code == 200 + + +async def test_aws_query_does_not_report_a_previous_attempts_request_id() -> None: + protocol = AwsQueryClientProtocol(_SERVICE_SCHEMA, "2020-01-08") + context = TypedProperties() + + first = HTTPResponse( + status=400, + fields=tuples_to_fields([]), + body=( + b"InvalidAction" + b"throttled" + b"attempt-1-id" + ), + ) + await _deserialize_query_error(protocol, first, context) + assert ( + protocol.extract_response_metadata(response=first, context=context).request_id + == "attempt-1-id" + ) + + second = HTTPResponse( + status=400, + fields=tuples_to_fields([]), + body=( + b"InvalidAction" + b"bad request" + ), + ) + await _deserialize_query_error(protocol, second, context) + + metadata = protocol.extract_response_metadata(response=second, context=context) + assert metadata.request_id is None + + +async def _deserialize_query_error( + protocol: AwsQueryClientProtocol, + response: HTTPResponse, + context: TypedProperties, +) -> None: + """Run an awsQuery error response through deserialization. + + Used to record whatever request ID the body carries the way a real call would, + rather than reaching into the protocol's private storage key. + """ + with pytest.raises(_ModeledQueryError): + await protocol.deserialize_response( + operation=_mock_operation( + _operation_schema("FailingOperation"), + error_schemas=[_INVALID_ACTION_ERROR_SCHEMA], + ), + request=cast(HTTPRequest, Mock()), + response=response, + error_registry=TypeRegistry( + {ShapeID("com.test#InvalidActionError"): _ModeledQueryError} + ), + context=context, + ) diff --git a/packages/smithy-aws-core/tests/unit/test_query.py b/packages/smithy-aws-core/tests/unit/test_query.py index 2c94ed980..f89e48f43 100644 --- a/packages/smithy-aws-core/tests/unit/test_query.py +++ b/packages/smithy-aws-core/tests/unit/test_query.py @@ -5,7 +5,9 @@ from typing import Any, cast from unittest.mock import Mock +import pytest from smithy_aws_core._private.query.errors import create_aws_query_error +from smithy_aws_core._private.query.metadata import parse_aws_query_request_id from smithy_aws_core._private.query.serializers import QueryShapeSerializer from smithy_core.documents import TypeRegistry from smithy_core.prelude import STRING @@ -321,3 +323,37 @@ def test_aws_query_error_retry_after_none_by_default() -> None: context=TypedProperties(), ) assert error.retry_after is None + + +@pytest.mark.parametrize( + "body, expected", + [ + # Successful responses nest the identifier under ResponseMetadata. + ( + b"" + b"abc-123" + b"", + "abc-123", + ), + # Error responses put it directly under the root element instead. + ( + b"InvalidGreeting" + b"foo-id", + "foo-id", + ), + # An empty element is treated as absent rather than as an empty ID. + (b"", None), + # An empty nested element falls through to the root rather than giving up. + ( + b"" + b"root-id", + "root-id", + ), + (b"x", None), + # A body that is not XML at all must not raise. + (b"not xml", None), + (b"", None), + ], +) +def test_parse_aws_query_request_id(body: bytes, expected: str | None) -> None: + assert parse_aws_query_request_id(body) == expected diff --git a/packages/smithy-aws-core/tests/unit/test_utils.py b/packages/smithy-aws-core/tests/unit/test_utils.py index 0606241b7..fab75a320 100644 --- a/packages/smithy-aws-core/tests/unit/test_utils.py +++ b/packages/smithy-aws-core/tests/unit/test_utils.py @@ -5,6 +5,7 @@ from smithy_aws_core.utils import ( parse_document_discriminator, parse_error_code, + parse_response_metadata, parse_retry_after, ) from smithy_core.documents import Document @@ -121,3 +122,95 @@ def test_parse_retry_after_ignores_standard_retry_after_header() -> None: fields=Fields([Field(name="Retry-After", values=["120"])]), ) assert parse_retry_after(response) is None + + +@pytest.mark.parametrize( + "headers, expected_request_id", + [ + # Most services send x-amzn-requestid. + ([("x-amzn-requestid", "rid-amzn")], "rid-amzn"), + # Services in the Amazon S3 lineage send x-amz-request-id instead. + ([("x-amz-request-id", "rid-amz")], "rid-amz"), + # When both are present, x-amzn-requestid takes precedence. + ( + [("x-amz-request-id", "rid-amz"), ("x-amzn-requestid", "rid-amzn")], + "rid-amzn", + ), + # An empty value is treated as absent rather than as an empty ID. + ([("x-amzn-requestid", "")], None), + # Falls through to the next candidate when the preferred one is empty. + ([("x-amzn-requestid", ""), ("x-amz-request-id", "rid-amz")], "rid-amz"), + ([], None), + ], +) +def test_parse_response_metadata_request_id( + headers: list[tuple[str, str]], expected_request_id: str | None +) -> None: + response = HTTPResponse( + status=200, + fields=Fields([Field(name=name, values=[value]) for name, value in headers]), + ) + assert parse_response_metadata(response).request_id == expected_request_id + + +@pytest.mark.parametrize( + "headers, expected", + [ + ([("x-amz-id-2", "host-id-2")], "host-id-2"), + ([("x-amz-id-2", "")], None), + ], +) +def test_parse_response_metadata_extended_request_id( + headers: list[tuple[str, str]], expected: str | None +) -> None: + response = HTTPResponse( + status=200, + fields=Fields([Field(name=name, values=[value]) for name, value in headers]), + ) + assert parse_response_metadata(response).extended_request_id == expected + + +def test_parse_response_metadata_reads_all_members() -> None: + response = HTTPResponse( + status=503, + fields=Fields( + [ + Field(name="x-amzn-requestid", values=["rid"]), + Field(name="x-amz-id-2", values=["host-id-2"]), + ] + ), + ) + metadata = parse_response_metadata(response) + assert metadata.request_id == "rid" + assert metadata.extended_request_id == "host-id-2" + assert metadata.http_status_code == 503 + + +def test_parse_response_metadata_ignores_unrelated_headers() -> None: + response = HTTPResponse( + status=200, + fields=Fields( + [ + Field(name="x-amz-retry-after", values=["100"]), + Field(name="request-id", values=["not-the-aws-header"]), + ] + ), + ) + metadata = parse_response_metadata(response) + assert metadata.request_id is None + assert metadata.extended_request_id is None + + +def test_parse_response_metadata_reads_the_first_of_repeated_values() -> None: + response = HTTPResponse( + status=200, + fields=Fields( + [ + Field(name="x-amzn-requestid", values=["rid-1", "rid-2"]), + Field(name="x-amz-id-2", values=["host-1", "host-2"]), + ] + ), + ) + metadata = parse_response_metadata(response) + assert metadata.request_id == "rid-1" + assert metadata.extended_request_id == "host-1" diff --git a/packages/smithy-core/.changes/next-release/smithy-core-breaking-ffcfa27938c44f9f9b839d62cde984bc.json b/packages/smithy-core/.changes/next-release/smithy-core-breaking-ffcfa27938c44f9f9b839d62cde984bc.json new file mode 100644 index 000000000..5f748c0eb --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-breaking-ffcfa27938c44f9f9b839d62cde984bc.json @@ -0,0 +1,4 @@ +{ + "type": "breaking", + "description": "Added `extract_response_metadata()` to the `ClientProtocol`. Implementations that extend `HttpClientProtocol` inherit a working one and need no change. Protocols that implement `ClientProtocol` directly must add it." +} diff --git a/packages/smithy-core/.changes/next-release/smithy-core-feature-b5b41df5a72d4340a396d624302bbbac.json b/packages/smithy-core/.changes/next-release/smithy-core-feature-b5b41df5a72d4340a396d624302bbbac.json new file mode 100644 index 000000000..464f83135 --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-feature-b5b41df5a72d4340a396d624302bbbac.json @@ -0,0 +1,4 @@ +{ + "type": "feature", + "description": "Added `ResponseMetadata`, exposing the request ID, extended request ID, and HTTP status code of the response that produced a result. It is attached to operation outputs and to modeled errors via a `response_metadata` attribute." +} diff --git a/packages/smithy-core/src/smithy_core/aio/client.py b/packages/smithy-core/src/smithy_core/aio/client.py index e668cb7de..bf0e05805 100644 --- a/packages/smithy-core/src/smithy_core/aio/client.py +++ b/packages/smithy-core/src/smithy_core/aio/client.py @@ -269,6 +269,8 @@ async def _execute_request[I: SerializeableShape, O: DeserializeableShape]( output_context = await self._handle_execution(call, request_future) output_context = self._finalize_execution(call, output_context) + self._attach_response_metadata(output_context) + if isinstance(output_context.response, Exception): e = output_context.response if not isinstance(e, SmithyError): @@ -277,6 +279,41 @@ async def _execute_request[I: SerializeableShape, O: DeserializeableShape]( return output_context.response, output_context # type: ignore + def _attach_response_metadata[I: SerializeableShape, O: DeserializeableShape]( + self, + output_context: OutputContext[I, O, TRequest | None, TResponse | None], + ) -> None: + """Attach metadata about the transport response to the result or error. + + This runs for successes and failures alike, so that request identifiers + stay available to callers for debugging. Results that do not carry the + attribute are left untouched, as are cases where no response was received + at all. + """ + result = output_context.response + transport_response = output_context.transport_response + + # Only reachable for errors raised before a response arrived, such as a + # connection timeout. The default empty metadata is left in place, where + # a null status code records that nothing came back. + if transport_response is None: + return + + if not hasattr(result, "response_metadata"): + return + + try: + metadata = self.protocol.extract_response_metadata( + response=transport_response, + context=output_context.properties, + ) + setattr(result, "response_metadata", metadata) + except Exception as e: + # Metadata is diagnostic and must never fail a call. Both statements + # above can raise: a broken protocol, or the assignment itself if the + # output shape is frozen. + _LOGGER.debug("Unable to attach response metadata: %s", e) + async def _handle_execution[I: SerializeableShape, O: DeserializeableShape]( self, call: ClientCall[I, O], @@ -378,7 +415,9 @@ async def _retry[I: SerializeableShape, O: DeserializeableShape]( and retry_error.retry_after is not None ): await sleep(retry_error.retry_after) - raise output_context.response + # Keeps the final attempt's response, so throttling and 5xx + # failures still carry a request ID when _execute_request raises. + return output_context _LOGGER.debug( "Retry needed. Attempting request #%s in %.4f seconds.", @@ -398,6 +437,9 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( request_future: Future[RequestContext[I, TRequest]] | None, ) -> OutputContext[I, O, TRequest, TResponse | None]: output_context: OutputContext[I, O, TRequest, TResponse | None] + # A modeled error arrives after the response does, so the response is kept + # here to report alongside it. Stays None if nothing came back. + transport_response: TResponse | None = None try: interceptor = call.interceptor interceptor.read_before_attempt(request_context) @@ -513,6 +555,7 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( response_context ), ) + transport_response = response_context.transport_response interceptor.read_before_deserialization(response_context) @@ -544,7 +587,7 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( request=request_context.request, response=e, transport_request=request_context.transport_request, - transport_response=None, + transport_response=transport_response, properties=request_context.properties, ) diff --git a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py index 146637ff5..83bbfe08b 100644 --- a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py +++ b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py @@ -14,6 +14,7 @@ from typing_extensions import TypeForm from ...deserializers import DeserializeableShape, ShapeDeserializer + from ...response import ResponseMetadata from ...schemas import APIOperation from ...serializers import SerializeableShape from ...shapes import ShapeID @@ -172,6 +173,28 @@ async def deserialize_response[ """ ... + def extract_response_metadata( + self, + *, + response: O, + context: TypedProperties, + ) -> "ResponseMetadata": + """Extract metadata about a transport response. + + This is called for both successful and failed invocations so that request + identifiers remain available to callers for debugging. Implementations + MUST NOT raise: metadata is diagnostic, so any value that cannot be + determined is left unset instead. + + :param response: The response to extract metadata from. + :param context: Per-call storage shared with this protocol's other methods. + Protocols whose request IDs arrive in headers need nothing from it, since + ``response`` already carries those. Protocols whose IDs arrive in the body + do: only ``deserialize_response`` sees the parsed body, so it must leave + the value here for this method to read back. + """ + ... + def create_event_publisher[ OperationInput: "SerializeableShape", OperationOutput: "DeserializeableShape", diff --git a/packages/smithy-core/src/smithy_core/exceptions.py b/packages/smithy-core/src/smithy_core/exceptions.py index 038cb4641..7459f3a81 100644 --- a/packages/smithy-core/src/smithy_core/exceptions.py +++ b/packages/smithy-core/src/smithy_core/exceptions.py @@ -3,6 +3,8 @@ from dataclasses import dataclass, field from typing import Literal +from .response import EMPTY_RESPONSE_METADATA, ResponseMetadata + class SmithyError(Exception): """Base exception type for all exceptions raised by smithy-python.""" @@ -54,6 +56,18 @@ class CallError(SmithyError): is_timeout_error: bool = False """Whether the error represents a timeout condition.""" + response_metadata: ResponseMetadata = field( + default=EMPTY_RESPONSE_METADATA, repr=False, compare=False + ) + """Metadata about the response that produced this error. + + Members of the metadata are individually optional, and an + ``http_status_code`` of ``None`` indicates that no response + was received at all. + + This is always set, so it is safe to access without a null check. + """ + def __post_init__(self): super().__init__(self.message) diff --git a/packages/smithy-core/src/smithy_core/response.py b/packages/smithy-core/src/smithy_core/response.py new file mode 100644 index 000000000..d975a1f83 --- /dev/null +++ b/packages/smithy-core/src/smithy_core/response.py @@ -0,0 +1,47 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +from dataclasses import dataclass + + +@dataclass(frozen=True, kw_only=True) +class ResponseMetadata: + """Metadata about the transport response that produced a result. + + This is attached to operation outputs and to errors so that callers can + recover the identifiers a service's support team needs in order to + investigate a request. + + Every member is optional, since the information available depends on the + protocol in use and on how far a call progressed before completing. In + particular, an ``http_status_code`` of ``None`` means that no response was + received at all, such as when a request timed out or an endpoint could not + be resolved. + """ + + request_id: str | None = None + """The service-assigned identifier for the request. + + This is the identifier that AWS support teams ask for when investigating a + case. + """ + + extended_request_id: str | None = None + """A secondary identifier for the request, used for debugging. + + Only some services return this. For AWS services it corresponds to the + ``x-amz-id-2`` header. + """ + + http_status_code: int | None = None + """The status code of the response. + + A value of ``None`` indicates that no response was received. + """ + + +EMPTY_RESPONSE_METADATA = ResponseMetadata() +"""Metadata used when no response information is available. + +Since :py:class:`ResponseMetadata` is immutable, this is shared rather than +allocated at each use. +""" diff --git a/packages/smithy-core/tests/unit/aio/_pipeline_harness.py b/packages/smithy-core/tests/unit/aio/_pipeline_harness.py index 16a43168e..487898465 100644 --- a/packages/smithy-core/tests/unit/aio/_pipeline_harness.py +++ b/packages/smithy-core/tests/unit/aio/_pipeline_harness.py @@ -6,10 +6,13 @@ from smithy_core import URI from smithy_core.aio.client import ClientCall, RequestPipeline from smithy_core.aio.interfaces import ClientProtocol +from smithy_core.aio.interfaces.retries import RetryStrategy +from smithy_core.aio.retries import SimpleRetryStrategy from smithy_core.deserializers import ShapeDeserializer from smithy_core.documents import TypeRegistry from smithy_core.endpoints import EndpointResolverParams from smithy_core.interceptors import InterceptorChain +from smithy_core.response import EMPTY_RESPONSE_METADATA, ResponseMetadata from smithy_core.schemas import APIOperation, Schema from smithy_core.serializers import ShapeSerializer from smithy_core.shapes import ShapeID, ShapeType @@ -41,6 +44,9 @@ def serialize(self, serializer: ShapeSerializer) -> None: class StubOutput: + # Declared the same way codegen declares it on generated operation outputs. + response_metadata: ResponseMetadata = EMPTY_RESPONSE_METADATA + @classmethod def deserialize(cls, deserializer: ShapeDeserializer) -> Self: return cls() @@ -55,6 +61,15 @@ def deserialize(cls, deserializer: ShapeDeserializer) -> Self: return cls() +_UNARY_INPUT_SCHEMA = Schema.collection( + id=ShapeID("com.example#UnaryInput"), + members={"message": {"target": _STRING}}, +) +_UNARY_OUTPUT_SCHEMA = Schema.collection( + id=ShapeID("com.example#UnaryOutput"), + members={"message": {"target": _STRING}}, +) + OPERATION = APIOperation( input=StubInput, output=StubOutput, @@ -69,6 +84,23 @@ def deserialize(cls, deserializer: ShapeDeserializer) -> Self: error_schemas=[], ) +# ``ClientCall.retryable()`` is False for operations with a streaming input, so +# OPERATION above never enters the retry loop. This one has no input stream and so +# exercises it. +UNARY_OPERATION = APIOperation( + input=StubInput, + output=StubOutput, + schema=Schema( + id=ShapeID("com.example#UnaryOperation"), + shape_type=ShapeType.OPERATION, + ), + input_schema=_UNARY_INPUT_SCHEMA, + output_schema=_UNARY_OUTPUT_SCHEMA, + error_registry=TypeRegistry({}), + effective_auth_schemes=[], + error_schemas=[], +) + class StubRequest: def __init__(self) -> None: @@ -115,6 +147,14 @@ def __init__(self) -> None: self.deserialize_response_calls = 0 self.create_event_publisher_calls = 0 self.create_event_receiver_calls = 0 + self.extract_response_metadata_calls = 0 + # What extraction yields; the values tests assert on. + self.stub_response_metadata = ResponseMetadata( + request_id="stub-request-id", + extended_request_id="stub-extended-request-id", + http_status_code=200, + ) + self.extract_response_metadata_error: Exception | None = None @property def id(self) -> ShapeID: @@ -132,6 +172,12 @@ async def deserialize_response(self, **kwargs: Any) -> StubOutput: self.deserialize_response_calls += 1 return StubOutput() + def extract_response_metadata(self, **kwargs: Any) -> ResponseMetadata: + self.extract_response_metadata_calls += 1 + if self.extract_response_metadata_error is not None: + raise self.extract_response_metadata_error + return self.stub_response_metadata + def create_event_publisher(self, **kwargs: Any) -> StubEventPublisher: self.create_event_publisher_calls += 1 return StubEventPublisher() @@ -194,6 +240,26 @@ def pipeline_harness(transport: UndeclaredTransport) -> PipelineHarness: return PipelineHarness(protocol=protocol, transport=transport, pipeline=pipeline) +def retryable_client_call( + retry_strategy: RetryStrategy | None = None, +) -> ClientCall[Any, Any]: + """A call that goes through the retry loop, unlike :py:func:`client_call`. + + Defaults to a strategy that allows a single attempt, so the loop gives up after + the first failure rather than sleeping through retries. + """ + return ClientCall( + input=StubInput(), + operation=UNARY_OPERATION, + context=TypedProperties(), + interceptor=InterceptorChain([]), + auth_scheme_resolver=StubAuthResolver(), + supported_auth_schemes={}, + endpoint_resolver=StubEndpointResolver(), + retry_strategy=retry_strategy or SimpleRetryStrategy(max_attempts=1), + ) + + def client_call() -> ClientCall[Any, Any]: return ClientCall( input=StubInput(), diff --git a/packages/smithy-core/tests/unit/aio/test_client.py b/packages/smithy-core/tests/unit/aio/test_client.py index e97e613e5..e1f69161f 100644 --- a/packages/smithy-core/tests/unit/aio/test_client.py +++ b/packages/smithy-core/tests/unit/aio/test_client.py @@ -1,9 +1,12 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 +from typing import Any + import pytest from smithy_core.aio.eventstream import DuplexEventStream, InputEventStream -from smithy_core.exceptions import UnsupportedTransportError +from smithy_core.exceptions import CallError, UnsupportedTransportError +from smithy_core.response import EMPTY_RESPONSE_METADATA from ._pipeline_harness import ( DuplexTransport, @@ -14,6 +17,7 @@ UndeclaredTransport, client_call, pipeline_harness, + retryable_client_call, ) @@ -63,3 +67,71 @@ async def test_input_stream_does_not_require_duplex_support() -> None: assert isinstance(stream, InputEventStream) assert isinstance(await stream.await_output(), StubOutput) + + +async def test_response_metadata_attached_to_output() -> None: + harness = pipeline_harness(NonDuplexTransport()) + + output = await harness.pipeline(client_call()) + + assert harness.protocol.extract_response_metadata_calls == 1 + assert output.response_metadata.request_id == "stub-request-id" + assert output.response_metadata.extended_request_id == "stub-extended-request-id" + assert output.response_metadata.http_status_code == 200 + + +async def test_response_metadata_attached_to_error() -> None: + harness = pipeline_harness(NonDuplexTransport()) + + async def raise_modeled_error(**kwargs: Any) -> StubOutput: + raise CallError("Rate exceeded") + + harness.protocol.deserialize_response = raise_modeled_error # type: ignore[method-assign] + + with pytest.raises(CallError) as exc_info: + await harness.pipeline(client_call()) + + assert exc_info.value.response_metadata.request_id == "stub-request-id" + assert exc_info.value.response_metadata.http_status_code == 200 + + +async def test_response_metadata_empty_when_no_response_received() -> None: + harness = pipeline_harness(NonDuplexTransport()) + + async def fail_to_send(**kwargs: Any) -> Any: + raise CallError("Connection failed") + + harness.transport.send = fail_to_send # type: ignore[method-assign] + + with pytest.raises(CallError) as exc_info: + await harness.pipeline(client_call()) + + assert exc_info.value.response_metadata is EMPTY_RESPONSE_METADATA + assert exc_info.value.response_metadata.http_status_code is None + assert harness.protocol.extract_response_metadata_calls == 0 + + +async def test_failed_metadata_extraction_does_not_fail_the_call() -> None: + harness = pipeline_harness(NonDuplexTransport()) + harness.protocol.extract_response_metadata_error = RuntimeError("boom") + + output = await harness.pipeline(client_call()) + + assert output.response_metadata is EMPTY_RESPONSE_METADATA + + +async def test_response_metadata_attached_when_retries_are_exhausted() -> None: + # Throttling and 5xx failures exit through the retry loop, which must carry + # the response out with the error or they report no request ID. + harness = pipeline_harness(NonDuplexTransport()) + + async def raise_retryable_error(**kwargs: Any) -> StubOutput: + raise CallError("Rate exceeded", is_retry_safe=True) + + harness.protocol.deserialize_response = raise_retryable_error # type: ignore[method-assign] + + with pytest.raises(CallError) as exc_info: + await harness.pipeline(retryable_client_call()) + + assert exc_info.value.response_metadata.request_id == "stub-request-id" + assert exc_info.value.response_metadata.http_status_code == 200 diff --git a/packages/smithy-http/.changes/next-release/smithy-http-feature-21a43ab4b58f4e2d90e0e3f59503f370.json b/packages/smithy-http/.changes/next-release/smithy-http-feature-21a43ab4b58f4e2d90e0e3f59503f370.json new file mode 100644 index 000000000..58c6ac645 --- /dev/null +++ b/packages/smithy-http/.changes/next-release/smithy-http-feature-21a43ab4b58f4e2d90e0e3f59503f370.json @@ -0,0 +1,4 @@ +{ + "type": "feature", + "description": "`HttpClientProtocol` implements `extract_response_metadata()`, reporting the HTTP status code." +} diff --git a/packages/smithy-http/src/smithy_http/aio/protocols.py b/packages/smithy-http/src/smithy_http/aio/protocols.py index 6bd37c17d..9a06aab4b 100644 --- a/packages/smithy-http/src/smithy_http/aio/protocols.py +++ b/packages/smithy-http/src/smithy_http/aio/protocols.py @@ -21,6 +21,7 @@ ) from smithy_core.interfaces import StreamingBlob as SyncStreamingBlob from smithy_core.prelude import DOCUMENT +from smithy_core.response import ResponseMetadata from smithy_core.schemas import APIOperation from smithy_core.serializers import SerializeableShape from smithy_core.shapes import ShapeID @@ -67,6 +68,19 @@ def set_service_endpoint( return request + def extract_response_metadata( + self, + *, + response: HTTPResponse, + context: TypedProperties, + ) -> ResponseMetadata: + """Extract the status code from an HTTP response. + + Identifiers such as request IDs are not part of HTTP itself, so protocols + that define them are expected to override this and add them. + """ + return ResponseMetadata(http_status_code=response.status) + class HttpBindingClientProtocol(HttpClientProtocol): """An HTTP-based protocol that uses HTTP binding traits.""" diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index cda2a79f8..af8386303 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -11,8 +11,9 @@ from smithy_core.interfaces import URI as URIInterface from smithy_core.schemas import APIOperation from smithy_core.shapes import ShapeID -from smithy_http import Fields -from smithy_http.aio import HTTPRequest +from smithy_core.types import TypedProperties as TypedPropertiesImpl +from smithy_http import Field, Fields +from smithy_http.aio import HTTPRequest, HTTPResponse from smithy_http.aio.interfaces import HTTPRequest as HTTPRequestInterface from smithy_http.aio.interfaces import HTTPResponse as HTTPResponseInterface from smithy_http.aio.protocols import HttpClientProtocol @@ -145,3 +146,21 @@ def test_http_protocol_joins_uris( updated_request = protocol.set_service_endpoint(request=request, endpoint=endpoint) actual = updated_request.destination assert actual == expected + + +def test_extract_response_metadata_reports_only_the_status_code() -> None: + # Request Ids are not an HTTP concept. The base HTTP layer must not read + # request IDs even when an AWS-style header is present; that knowledge + # belongs in smithy-aws-core. + response = HTTPResponse( + status=429, + fields=Fields([Field(name="x-amzn-requestid", values=["rid"])]), + ) + + metadata = MockProtocol().extract_response_metadata( + response=response, context=TypedPropertiesImpl() + ) + + assert metadata.http_status_code == 429 + assert metadata.request_id is None + assert metadata.extended_request_id is None