diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 4a10a0913e..6c92ef896b 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -55,6 +55,12 @@ references = ["smithy-rs#2948"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all" } author = "Velfi" +[[aws-sdk-rust]] +message = "Correctly identify HTTP 200 responses from S3 with `` as the root Element as errors. **Note**: This a behavior change and will change the error type returned by the SDK in some cases." +references = ["smithy-rs#2958", "aws-sdk-rust#873"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "rcoh" + [[aws-sdk-rust]] message = "Allow `no_credentials` to be used with all S3 operations." references = ["smithy-rs#2955", "aws-sdk-rust#878"] diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3Decorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3Decorator.kt index 31f134d0bb..a5610f0ef0 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3Decorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/s3/S3Decorator.kt @@ -23,7 +23,9 @@ import software.amazon.smithy.rust.codegen.client.smithy.ClientRustSettings import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator import software.amazon.smithy.rust.codegen.client.smithy.endpoint.EndpointCustomization import software.amazon.smithy.rust.codegen.client.smithy.endpoint.rustName +import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationGenerator +import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationSection import software.amazon.smithy.rust.codegen.client.smithy.protocols.ClientRestXmlFactory import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate @@ -106,6 +108,34 @@ class S3Decorator : ClientCodegenDecorator { ) } + override fun operationCustomizations( + codegenContext: ClientCodegenContext, + operation: OperationShape, + baseCustomizations: List, + ): List { + return baseCustomizations + object : OperationCustomization() { + override fun section(section: OperationSection): Writable { + return writable { + when (section) { + is OperationSection.BeforeParseResponse -> { + section.body?.also { body -> + rustTemplate( + """ + if matches!(#{errors}::body_is_error($body), Ok(true)) { + ${section.forceError} = true; + } + """, + "errors" to RuntimeType.unwrappedXmlErrors(codegenContext.runtimeConfig), + ) + } + } + else -> {} + } + } + } + } + } + private fun isInInvalidXmlRootAllowList(shape: Shape): Boolean { return shape.isStructureShape && invalidXmlRootAllowList.contains(shape.id) } @@ -115,7 +145,7 @@ class FilterEndpointTests( private val testFilter: (EndpointTestCase) -> EndpointTestCase? = { a -> a }, private val operationInputFilter: (EndpointTestOperationInput) -> EndpointTestOperationInput? = { a -> a }, ) { - fun updateEndpointTests(endpointTests: List): List { + private fun updateEndpointTests(endpointTests: List): List { val filteredTests = endpointTests.mapNotNull { test -> testFilter(test) } return filteredTests.map { test -> val operationInputs = test.operationInputs @@ -123,7 +153,7 @@ class FilterEndpointTests( } } - fun transform(model: Model) = ModelTransformer.create().mapTraits(model) { _, trait -> + fun transform(model: Model): Model = ModelTransformer.create().mapTraits(model) { _, trait -> when (trait) { is EndpointTestsTrait -> EndpointTestsTrait.builder().testCases(updateEndpointTests(trait.testCases)) .version(trait.version).build() @@ -135,7 +165,7 @@ class FilterEndpointTests( // TODO(P96049742): This model transform may need to change depending on if and how the S3 model is updated. private class AddOptionalAuth { - fun transform(model: Model) = ModelTransformer.create().mapShapes(model) { shape -> + fun transform(model: Model): Model = ModelTransformer.create().mapShapes(model) { shape -> // Add @optionalAuth to all S3 operations if (shape is OperationShape && !shape.hasTrait()) { shape.toBuilder() diff --git a/aws/sdk/integration-tests/s3/tests/status-200-errors.rs b/aws/sdk/integration-tests/s3/tests/status-200-errors.rs new file mode 100644 index 0000000000..5fee498ec6 --- /dev/null +++ b/aws/sdk/integration-tests/s3/tests/status-200-errors.rs @@ -0,0 +1,40 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use aws_credential_types::provider::SharedCredentialsProvider; +use aws_credential_types::Credentials; +use aws_sdk_s3::Client; +use aws_smithy_client::test_connection::infallible_connection_fn; +use aws_smithy_http::body::SdkBody; +use aws_smithy_types::error::metadata::ProvideErrorMetadata; +use aws_types::region::Region; +use aws_types::SdkConfig; + +const ERROR_RESPONSE: &str = r#" + + SlowDown + Please reduce your request rate. + K2H6N7ZGQT6WHCEG + WWoZlnK4pTjKCYn6eNV7GgOurabfqLkjbSyqTvDMGBaI9uwzyNhSaDhOCPs8paFGye7S6b/AB3A= + +"#; + +#[tokio::test] +async fn status_200_errors() { + let conn = infallible_connection_fn(|_req| http::Response::new(SdkBody::from(ERROR_RESPONSE))); + let sdk_config = SdkConfig::builder() + .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests())) + .region(Region::new("us-west-4")) + .http_connector(conn) + .build(); + let client = Client::new(&sdk_config); + let error = client + .delete_objects() + .bucket("bucket") + .send() + .await + .expect_err("should fail"); + assert_eq!(error.into_service_error().code(), Some("SlowDown")); +} diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt index 7088dbdb02..d872311cd6 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationCustomization.kt @@ -48,6 +48,14 @@ sealed class OperationSection(name: String) : Section(name) { data class BeforeParseResponse( override val customizations: List, val responseName: String, + /** + * Name of the `force_error` variable. Set this to true to trigger error parsing. + */ + val forceError: String, + /** + * When set, the name of the response body data field + */ + val body: String?, ) : OperationSection("BeforeParseResponse") /** diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt index 0f68a27603..6a04bb0c4a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/protocol/ResponseDeserializerGenerator.kt @@ -94,10 +94,12 @@ class ResponseDeserializerGenerator( rustTemplate( """ fn deserialize_streaming(&self, response: &mut #{HttpResponse}) -> #{Option}<#{OutputOrError}> { + ##[allow(unused_mut)] + let mut force_error = false; #{BeforeParseResponse} // If this is an error, defer to the non-streaming parser - if !response.status().is_success() && response.status().as_u16() != $successCode { + if (!response.status().is_success() && response.status().as_u16() != $successCode) || force_error { return #{None}; } #{Some}(#{type_erase_result}(#{parse_streaming_response}(response))) @@ -106,7 +108,7 @@ class ResponseDeserializerGenerator( *codegenScope, "parse_streaming_response" to parserGenerator.parseStreamingResponseFn(operationShape, customizations), "BeforeParseResponse" to writable { - writeCustomizations(customizations, OperationSection.BeforeParseResponse(customizations, "response")) + writeCustomizations(customizations, OperationSection.BeforeParseResponse(customizations, "response", "force_error", body = null)) }, ) } @@ -136,8 +138,10 @@ class ResponseDeserializerGenerator( let (success, status) = (response.status().is_success(), response.status().as_u16()); let headers = response.headers(); let body = response.body().bytes().expect("body loaded"); + ##[allow(unused_mut)] + let mut force_error = false; #{BeforeParseResponse} - let parse_result = if !success && status != $successCode { + let parse_result = if !success && status != $successCode || force_error { #{parse_error}(status, headers, body) } else { #{parse_response}(status, headers, body) @@ -148,7 +152,7 @@ class ResponseDeserializerGenerator( "parse_error" to parserGenerator.parseErrorFn(operationShape, customizations), "parse_response" to parserGenerator.parseResponseFn(operationShape, customizations), "BeforeParseResponse" to writable { - writeCustomizations(customizations, OperationSection.BeforeParseResponse(customizations, "response")) + writeCustomizations(customizations, OperationSection.BeforeParseResponse(customizations, "response", "force_error", "body")) }, ) }