-
Notifications
You must be signed in to change notification settings - Fork 24
Add error serde to http-binding client protocols #466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,14 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| from smithy_core.shapes import ShapeID | ||
| from smithy_http.aio.interfaces import ErrorExtractor, HTTPResponse | ||
|
|
||
|
|
||
| class AmznErrorExtractor(ErrorExtractor): | ||
| """Attempts to extract the Amazon-specific 'X-Amzn-Errortype' error header from a | ||
| response.""" | ||
|
|
||
| def get_error(self, response: HTTPResponse): | ||
| if "x-amzn-errortype" in response.fields: | ||
| val = response.fields["x-amzn-errortype"].values[0] | ||
| return ShapeID(val) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| from dataclasses import dataclass | ||
| from typing import Literal | ||
|
|
||
| from smithy_core.deserializers import DeserializeableShape | ||
|
|
||
| type Fault = Literal["client", "server", "other"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. other? |
||
|
|
||
|
|
||
| @dataclass(kw_only=True, frozen=True) | ||
| class CallException(RuntimeError): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All exceptions we define should inherit from |
||
| """The top-level exception that should be used to throw application-level errors | ||
| from clients and servers. | ||
| This should be used in protocol error deserialization, throwing errors based on | ||
| protocol-hints, network errors, and shape validation errors. It should not be used | ||
| for illegal arguments, null argument validation, or other kinds of logic errors | ||
| sufficiently covered by the Java standard library. | ||
| """ | ||
|
|
||
| fault: Fault = "other" | ||
| """The party that is at fault for the error, if any.""" | ||
|
|
||
| message: str = "" | ||
| """The error message.""" | ||
|
|
||
| # TODO: retry-ability and associated information (throttling, duration, etc.), perhaps 'Retryability' dataclass? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah having a |
||
|
|
||
|
|
||
| @dataclass(kw_only=True, frozen=True) | ||
| class ModeledException(CallException, DeserializeableShape): | ||
| """The top-level exception that should be used to throw modeled errors from clients | ||
| and servers.""" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| from smithy_core.aio.interfaces import ClientTransport, Request, Response | ||
| from smithy_core.aio.utils import read_streaming_blob, read_streaming_blob_async | ||
| from smithy_core.shapes import ShapeID | ||
|
|
||
| from ...interfaces import ( | ||
| Fields, | ||
|
|
@@ -83,3 +84,18 @@ async def send( | |
| :param request_config: Configuration specific to this request. | ||
| """ | ||
| ... | ||
|
|
||
|
|
||
| class ErrorExtractor(Protocol): | ||
| """Extract error shape IDs from an HTTP response.""" | ||
|
|
||
| def get_error( | ||
| self, | ||
| response: HTTPResponse, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't be tied to HTTP |
||
| ) -> ShapeID | None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might actually be the place to get the retry info |
||
| """Get the shape id for an error by using information (such as headers) from a | ||
| response. | ||
|
|
||
| :param response: The response object to derive an error shape from. | ||
| """ | ||
| ... | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,19 @@ | |
| import datetime | ||
| from collections.abc import Callable | ||
| from decimal import Decimal | ||
| from typing import TYPE_CHECKING | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from smithy_core.codecs import Codec | ||
| from smithy_core.deserializers import ShapeDeserializer, SpecificShapeDeserializer | ||
| from smithy_core.exceptions import UnsupportedStreamException | ||
| from smithy_core.interfaces import is_bytes_reader, is_streaming_blob | ||
| from smithy_core.schemas import Schema | ||
| from smithy_core.documents import TypeRegistry | ||
| from smithy_core.errors import CallException, ModeledException | ||
| from smithy_core.exceptions import ( | ||
| ExpectationNotMetException, | ||
| UnsupportedStreamException, | ||
| ) | ||
| from smithy_core.interfaces import TypedProperties, is_bytes_reader, is_streaming_blob | ||
| from smithy_core.prelude import DOCUMENT | ||
| from smithy_core.schemas import APIOperation, Schema | ||
| from smithy_core.shapes import ShapeType | ||
| from smithy_core.traits import ( | ||
| HTTPHeaderTrait, | ||
|
|
@@ -22,15 +28,15 @@ | |
| from smithy_core.types import TimestampFormat | ||
| from smithy_core.utils import ensure_utc, strict_parse_bool, strict_parse_float | ||
|
|
||
| from .aio.interfaces import HTTPResponse | ||
| from .interfaces import Field, Fields | ||
| from smithy_http.aio.interfaces import ErrorExtractor, HTTPResponse | ||
| from smithy_http.interfaces import Field, Fields | ||
|
|
||
| if TYPE_CHECKING: | ||
| from smithy_core.aio.interfaces import StreamingBlob as AsyncStreamingBlob | ||
| from smithy_core.interfaces import StreamingBlob as SyncStreamingBlob | ||
|
|
||
|
|
||
| __all__ = ["HTTPResponseDeserializer"] | ||
| __all__ = ["HTTPErrorDeserializer", "HTTPResponseDeserializer"] | ||
|
|
||
|
|
||
| class HTTPResponseDeserializer(SpecificShapeDeserializer): | ||
|
|
@@ -257,3 +263,72 @@ def _consume_payload(self) -> bytes: | |
| "Unable to read async stream. This stream must be buffered prior " | ||
| "to creating the deserializer." | ||
| ) | ||
|
|
||
|
|
||
| class HTTPErrorDeserializer: | ||
| """Binds an error response to a modelled or unknown exception.""" | ||
|
|
||
| def __init__( | ||
| self, | ||
| payload_codec: Codec, | ||
| extractor: ErrorExtractor, | ||
| response: HTTPResponse, | ||
| body: "SyncStreamingBlob", | ||
| ) -> None: | ||
| """Initialize an HTTPErrorDeserializer. | ||
|
|
||
| :param payload_codec: The Codec to use to deserialize the payload, if present. | ||
| :param extractor: The error extractor to get error shape id from the response. | ||
| :param response: The HTTP response to read from. | ||
| :param body: The HTTP response body in a synchronously readable form. This is | ||
| necessary for async response bodies when there is no streaming member. | ||
| """ | ||
| self._payload_codec = payload_codec | ||
| self._response = response | ||
| self._body = body | ||
| self._extractor = extractor | ||
| self._codec = payload_codec | ||
|
|
||
| def read_error( | ||
| self, | ||
| operation: APIOperation[Any, Any], | ||
| error_registry: TypeRegistry, | ||
| context: TypedProperties, | ||
| ) -> CallException: | ||
| body = self._body | ||
| if isinstance(body, bytearray): | ||
| body = bytes(body) | ||
| deserializer = self._payload_codec.create_deserializer(body) | ||
| document = deserializer.read_document(DOCUMENT) | ||
|
Comment on lines
+301
to
+302
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can errors have streaming payloads? (blob streams that is) |
||
|
|
||
| # try to get the error shape-id from the extractor | ||
| error_id = self._extractor.get_error(self._response) | ||
|
|
||
| # if none, get it from the parsed document (e.g. '__type') | ||
| if error_id is None: | ||
| error_id = document.discriminator | ||
|
Comment on lines
+307
to
+309
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're getting this document from the payload codec, but that's not necessarily gonna give you what you want. Different JSON protocols could embed this differently. |
||
|
|
||
| if error_id is not None: | ||
| error_shape = error_registry.get(error_id) | ||
| # make sure the error shape is derived from modeled exception | ||
| if not isinstance(error_shape, ModeledException): | ||
| raise ExpectationNotMetException( | ||
| f"Modeled errors must be derived from 'ModeledException', but got {error_shape}" | ||
| ) | ||
|
|
||
| # return the deserialized error | ||
| return error_shape.deserialize(deserializer) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to rewind the body since you've already read from it |
||
|
|
||
| # unknown error (no header, no type/unrecognized type) | ||
| fault = "other" | ||
| if 400 <= self._response.status < 500: | ||
| fault = "client" | ||
| elif self._response.status >= 500: | ||
| fault = "server" | ||
| message = ( | ||
| f"Unknown error: {operation.output_schema.id} " | ||
| f"- code: {self._response.status} " | ||
| f"- reason: {self._response.reason}" | ||
| ) | ||
|
|
||
| return CallException(message=message, fault=fault) | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will be enough. This can show up in a few other headers and has several formats which may or may not include a namespace. I could also be in the body.