Add error serde to http-binding client protocols #466
Conversation
| def get_error(self, response: HTTPResponse): | ||
| if "x-amzn-errortype" in response.fields: | ||
| val = response.fields["x-amzn-errortype"].values[0] | ||
| return ShapeID(val) |
There was a problem hiding this comment.
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.
|
|
||
|
|
||
| @dataclass(kw_only=True, frozen=True) | ||
| class CallException(RuntimeError): |
There was a problem hiding this comment.
All exceptions we define should inherit from SmithyException
|
|
||
| from smithy_core.deserializers import DeserializeableShape | ||
|
|
||
| type Fault = Literal["client", "server", "other"] |
| message: str = "" | ||
| """The error message.""" | ||
|
|
||
| # TODO: retry-ability and associated information (throttling, duration, etc.), perhaps 'Retryability' dataclass? |
There was a problem hiding this comment.
Yeah having a RetryableException protocol, mixin, or something would be ideal. Classifying is too complex without embedding the information in the exception.
|
|
||
| def get_error( | ||
| self, | ||
| response: HTTPResponse, |
There was a problem hiding this comment.
This shouldn't be tied to HTTP
| def get_error( | ||
| self, | ||
| response: HTTPResponse, | ||
| ) -> ShapeID | None: |
There was a problem hiding this comment.
This might actually be the place to get the retry info
| deserializer = self._payload_codec.create_deserializer(body) | ||
| document = deserializer.read_document(DOCUMENT) |
There was a problem hiding this comment.
Can errors have streaming payloads? (blob streams that is)
| ) | ||
|
|
||
| # return the deserialized error | ||
| return error_shape.deserialize(deserializer) |
There was a problem hiding this comment.
You need to rewind the body since you've already read from it
| # if none, get it from the parsed document (e.g. '__type') | ||
| if error_id is None: | ||
| error_id = document.discriminator |
There was a problem hiding this comment.
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.
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.