Don't use the value of AwsQueryError in json rpc/smithy-rpc-v2-cbor protocols

[alextwoods]

Don't use the value of AwsQueryError in json rpc/smithy-rpc-v2-cbor protocols

Motivation and Context

The AwsQueryError trait (in our C2J models, error) MUST apply only to query and rest json/xml protocols. It MUST NOT be applied to json (rpc) and smithy-rpcv2-cbor protocols.

Right now a json or smithy-rpcv2-cbor service with error traits will incorrectly deserialize all errors as generic service exceptions and NOT as the correct, modeled exception class.

Modifications

• When resolving the error code for an exception during codegen, check if the protocol supports the error trait before using it's value. Note: this changes only which exception class is used when deserializing errors. It does NOT change how the awsQueryCompatible trait is applied at runtime. That is done in the AwsJsonProtocolErrorUnmarshaller#getEffectiveErrorCode method
• Made protocol test error assertions more strict. The protocol tests now assert the expected class of the exception matches as well.
• Ensure rpc 1.0/1.1 error code parsing matches smithy spec: use both __type and code fields and handle uris in error codes (Fixes protocol test case failures).

Testing

Added a new integration test + increased strictness of existing protocol tests.

Why is this change safe to make?
There are no existing json or smithy-rpcv2-cbor service models in the repo with the error trait - so this should not change the behavior of any current services.

Note: this changes only which exception class is used when deserializing errors. It does NOT change how the awsQueryCompatible trait is applied at runtime. That is done in the AwsJsonProtocolErrorUnmarshaller#getEffectiveErrorCode method

Why did protocol tests not catch this?
Our protocol tests previously only checked that the deserialized error code (and other fields) on the exception were correct. They were NOT asserting that the class of the exception matched the expected class.

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install succeeds
• My code follows the code style of this project
• I have added tests to cover my changes
• All new and existing tests passed
• I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.

License

• I confirm that this pull request can be released under the Apache 2 license

[zoewangg]

nit, can we create unit tests with mock server and mock response instead to make the test more reliable?

[alextwoods]

That was my first thought, but the issue with using a mock response is that it needs to be protocol specific, so when we switch protocols the test breaks and additionally we have an existing integration test that (testExceptionHandling - just above this one) that tests something similar, so this shouldn't introduce a new sort of flaky test. I could however, roll the stricter assertions from my new test into the testExceptionHandling to avoid adding a new test?

Of course - now that we have switched to rpcv2-cbor, I don't know if we'd ever switch protocols again so having a protocol specific unit test probably doesn't matter?

The test I added was originally added to the internal cloudwatch package to ensure compatibility with the query->cbor migration and is what caught this issue in the first place.