Skip to content
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

feat: add support for awsQueryCompatible trait #1810

Merged
merged 6 commits into from
Nov 5, 2024
Merged

Conversation

dayaffe
Copy link
Collaborator

@dayaffe dayaffe commented Nov 4, 2024

Related to smithy-lang/smithy-swift#850

Issue #

SWIFT-1865

Description of changes

  • If a service is marked with awsQueryCompatible trait then requests will send an additional header x-amzn-query-mode set to true. This will tell the service to send back the x-amzn-query-error in response with error code and type information
  • Since we do not currently expose Type/Fault we will not add it and continue to only expose Code
  • If a service is marked with awsQueryCompatible trait operations will parse the value of the x-amzn-query-error and set the base error appropriate. This currently only applies to SQS which uses aws json 1_0 and thus only applies to AWSJsonError class.
  • Create class AwsQueryCompatibleErrorDetails with unit tests to handle logic for parsing x-amzn-query-error
  • Integration test is added to check for correct errorCode

New/existing dependencies impact assessment, if applicable

Conventional Commits

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -11,7 +11,7 @@ import class SmithyHTTPAPI.HTTPResponse
@_spi(SmithyReadWrite) import class SmithyJSON.Reader

public struct AWSJSONError: BaseError {
public let code: String
public var code: String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be set after the AWSJSONError is constructed? Can't we pass errorDetails (or nil if not used) into the constructor? Alternatively can we roll the awsQueryCompatible logic into this constructor & pass in a Bool to indicate whether that trait is present? (The purpose of the BaseError type is to encapsulate protocol-specific error logic like this.)

That would allow us to keep this property immutable. Swift properties should generally be immutable unless there is a reason it must be changed after initialization.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't feel right about changing the constructor for the general AWSJsonError just for a service customization used only by SQS. Will think if there's a better way to keep this as a constant

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have code passed in as an optional argument and then handle setting code in the extension function

@dayaffe dayaffe requested a review from jbelkins November 5, 2024 18:29
@dayaffe dayaffe merged commit 0337c73 into main Nov 5, 2024
29 checks passed
@dayaffe dayaffe deleted the day/query-compat branch November 5, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants