feat: cbor protocol support

[stobrien89]

Description of changes:
Adds support for the Smithy RPC V2 CBOR protocol.

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

[yenfryherrerafeliz]

Some comments and a general question, why not moving the Cbor folder under the Api folder?, since it is most likely going to be used by implementations under Api.

[yenfryherrerafeliz]

Why resolving the error code again when it was already resolved with the same logic here?

[stobrien89]

This was just logic ported over from another query compatible error parser, but moved the case change to the extractErrorCode() call and removed this redundant extraction

[yenfryherrerafeliz]

The condition looks for bodies that are either non-seekable or has a size, but, in the case the body is not seekable, we should take into account that the body can be read just once, which means any other logic forward that depends on reading the body again will fails or produce unexpected behaviors. I addressed something similar on this PR.

[stobrien89]

I've removed this for now and added a TODO for when changes get applied to EventParsingIterator to decouple it from json and abstract to the protocol error parsers. For now, we can reasonably only expect seekable bodies from services that support CBOR (no more than 3)

[yenfryherrerafeliz]

I think you should either define a default value for $smithyProtocol or make sure the variable is not null. This could cause undesired behaviors if for any reason the implementation omits overriding this property and assigning a value to it. Another option could be to define an static abstract method to force implementators to provide a value, but this is only if the value for this property will be different based on the parser, otherwise I suggest using a const.

[stobrien89]

this was intentional (and documented in the class docblock). Implementers must define the protocol- it's self-enforcing

[yenfryherrerafeliz]

I think an abstract static method would make more sense here, since I understand it is well documented, which is good, but there is not overriding enforcement here. For this looks error prone, since implementators could omit setting a value to it and the issue will be caught at runtime. However, I don`t see this as a blocker since current implementation will not fail.

[stobrien89]

I considered that, but I see it like this: an implementer will not get very far in testing something as crucial as a serializer or parser before discovering something like this, and in that case, an enforcement function is not necessary. If this was a lightweight utility that wasn't an internal component, I'd probably go that route

[yenfryherrerafeliz]

I know allowing dependency injection is good for testing purposes, however, in this scenario I dont see the need of allowing this property to be passed in, unless the type annotated here is either, a class that can be extended/mocked, an interface, or an abstract class. However, in this case CborDecoder is a final class and can't be customized at all. From my point of view I think is safe to just initialize internally.

What do yo think?

[stobrien89]

I agree that it seems pointless- my initial thought was that some dependency injection was better than none, but making this extensible would be a footgun, and dependency injection here is unnecessary complexity. I'll initialize internally

[yenfryherrerafeliz]

Looks good. Just a minor comment, but is optional to address regarding enforcing the $smithyProtocol override.

[yenfryherrerafeliz]

I think an abstract static method would make more sense here, since I understand it is well documented, which is good, but there is not overriding enforcement here. For this looks error prone, since implementators could omit setting a value to it and the issue will be caught at runtime. However, I don`t see this as a blocker since current implementation will not fail.