Deserialize Extended S3 Errors (#429)

* Add customization for S3 host ID

* Cleanup and fully replace the s3 protocol

* Only generate the customization for S3

* Fix bugs which caused rustfmt to crash

* Add test

* Back out unused change

* Update aws/sdk/integration-tests/s3/Cargo.toml

* CR feedback, add missing test
This commit is contained in:
Russell Cohen 2021-05-28 12:55:07 -04:00 committed by GitHub
parent 362e3440c8
commit a9b6e6e1a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 347 additions and 58 deletions

View File

@ -0,0 +1,14 @@
[package]
name = "inlineable-aws"
version = "0.1.0"
authors = ["Russell Cohen <rcoh@amazon.com>"]
edition = "2018"
description = """
The modules of this crate are intended to be inlined directly into the SDK as needed. The dependencies here
are to allow this crate to be compilable and testable in isolation, no client code actually takes these dependencies.
"""
[dependencies]
smithy-xml = { path = "../../../rust-runtime/smithy-xml" }
smithy-types = { path = "../../../rust-runtime/smithy-types" }
http = "0.2.4"

View File

@ -0,0 +1,7 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
#[allow(dead_code)]
mod s3_errors;

View File

@ -0,0 +1,72 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
const EXTENDED_REQUEST_ID: &str = "s3_extended_request_id";
pub trait ErrorExt {
fn extended_request_id(&self) -> Option<&str>;
}
impl ErrorExt for smithy_types::Error {
fn extended_request_id(&self) -> Option<&str> {
self.extra(EXTENDED_REQUEST_ID)
}
}
pub fn parse_extended_error<B>(
error: smithy_types::Error,
response: &http::Response<B>,
) -> smithy_types::Error {
let mut builder = error.into_builder();
let host_id = response
.headers()
.get("x-amz-id-2")
.and_then(|header_value| header_value.to_str().ok());
if let Some(host_id) = host_id {
builder.custom(EXTENDED_REQUEST_ID, host_id);
}
builder.build()
}
#[cfg(test)]
mod test {
use crate::s3_errors::{parse_extended_error, ErrorExt};
#[test]
fn add_error_fields() {
let resp = http::Response::builder()
.header(
"x-amz-id-2",
"eftixk72aD6Ap51TnqcoF8eFidJG9Z/2mkiDFu8yU9AS1ed4OpIszj7UDNEHGran",
)
.status(400)
.body("")
.unwrap();
let error = smithy_types::Error::builder()
.message("123")
.request_id("456")
.build();
let error = parse_extended_error(error, &resp);
assert_eq!(
error
.extended_request_id()
.expect("extended request id should be set"),
"eftixk72aD6Ap51TnqcoF8eFidJG9Z/2mkiDFu8yU9AS1ed4OpIszj7UDNEHGran"
);
}
#[test]
fn handle_missing_header() {
let resp = http::Response::builder().status(400).body("").unwrap();
let error = smithy_types::Error::builder()
.message("123")
.request_id("456")
.build();
let error = parse_extended_error(error, &resp);
assert_eq!(error.extended_request_id(), None);
}
}

View File

@ -0,0 +1,20 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
description = "Rust Runtime"
plugins {
kotlin("jvm")
}
group = "software.amazon.aws.rustruntime"
version = "0.0.3"
tasks.jar {
from("./") {
include("aws-inlineable/src/*.rs")
include("aws-inlineable/Cargo.toml")
}
}

View File

@ -21,6 +21,7 @@ val kotestVersion: String by project
dependencies {
implementation(project(":codegen"))
runtimeOnly(project(":aws:rust-runtime"))
implementation("software.amazon.smithy:smithy-protocol-test-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
testImplementation("org.junit.jupiter:junit-jupiter:5.6.1")

View File

@ -6,7 +6,8 @@
package software.amazon.smithy.rustsdk
import software.amazon.smithy.rust.codegen.smithy.customize.CombinedCodegenDecorator
import software.amazon.smithy.rustsdk.customize.apigateway.ApiGatewayCustomizationDecorator
import software.amazon.smithy.rustsdk.customize.apigateway.ApiGatewayDecorator
import software.amazon.smithy.rustsdk.customize.s3.S3Decorator
val DECORATORS = listOf(
CredentialsProviderDecorator(),
@ -17,8 +18,9 @@ val DECORATORS = listOf(
RetryPolicyDecorator(),
IntegrationTestDecorator(),
FluentClientDecorator(),
ApiGatewayCustomizationDecorator(),
CrateLicenseDecorator()
ApiGatewayDecorator(),
CrateLicenseDecorator(),
S3Decorator()
)
class AwsCodegenDecorator : CombinedCodegenDecorator(DECORATORS) {

View File

@ -6,25 +6,31 @@
package software.amazon.smithy.rustsdk
import software.amazon.smithy.rust.codegen.rustlang.CargoDependency
import software.amazon.smithy.rust.codegen.rustlang.Local
import software.amazon.smithy.rust.codegen.smithy.RuntimeConfig
import software.amazon.smithy.rust.codegen.smithy.RuntimeCrateLocation
import software.amazon.smithy.rust.codegen.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.smithy.crateLocation
import java.io.File
import java.nio.file.Path
fun RuntimeConfig.awsRoot(): String {
check(runtimeCrateLocation is RuntimeCrateLocation.Path) { "cannot run tests on versioned runtime dependencies" }
val cratePath = (runtimeCrateLocation as RuntimeCrateLocation.Path).path
val asPath = Path.of(cratePath)
val path = if (asPath.isAbsolute) {
asPath.parent.resolve("aws/rust-runtime").toAbsolutePath().toString()
} else {
cratePath
fun RuntimeConfig.awsRoot(): RuntimeCrateLocation = when (runtimeCrateLocation) {
is RuntimeCrateLocation.Path -> {
val cratePath = (runtimeCrateLocation as RuntimeCrateLocation.Path).path
val asPath = Path.of(cratePath)
val path = if (asPath.isAbsolute) {
asPath.parent.resolve("aws/rust-runtime").toAbsolutePath().toString()
} else {
cratePath
}
check(File(path).exists()) { "$path must exist to generate a working SDK" }
RuntimeCrateLocation.Path(path)
}
check(File(path).exists()) { "$path must exist to generate a working SDK" }
return path
is RuntimeCrateLocation.Versioned -> runtimeCrateLocation
}
object AwsRuntimeType {
val S3Errors by lazy { RuntimeType.forInlineDependency(InlineAwsDependency.forRustFile("s3_errors")) }
}
fun RuntimeConfig.awsRuntimeDependency(name: String, features: List<String> = listOf()): CargoDependency =
CargoDependency(name, Local(awsRoot()), features = features)
CargoDependency(name, awsRoot().crateLocation(), features = features)

View File

@ -0,0 +1,12 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
package software.amazon.smithy.rustsdk
import software.amazon.smithy.rust.codegen.rustlang.InlineDependency
object InlineAwsDependency {
fun forRustFile(file: String): InlineDependency = InlineDependency.Companion.forRustFile(file, "aws-inlineable")
}

View File

@ -17,7 +17,7 @@ import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator
import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolConfig
import software.amazon.smithy.rust.codegen.smithy.letIf
class ApiGatewayCustomizationDecorator : RustCodegenDecorator {
class ApiGatewayDecorator : RustCodegenDecorator {
override val name: String = "ApiGateway"
override val order: Byte = 0

View File

@ -0,0 +1,86 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
package software.amazon.smithy.rustsdk.customize.s3
import software.amazon.smithy.aws.traits.protocols.RestXmlTrait
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.rust.codegen.rustlang.CargoDependency
import software.amazon.smithy.rust.codegen.rustlang.Writable
import software.amazon.smithy.rust.codegen.rustlang.asType
import software.amazon.smithy.rust.codegen.rustlang.rust
import software.amazon.smithy.rust.codegen.rustlang.rustBlockTemplate
import software.amazon.smithy.rust.codegen.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.rustlang.writable
import software.amazon.smithy.rust.codegen.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator
import software.amazon.smithy.rust.codegen.smithy.generators.LibRsCustomization
import software.amazon.smithy.rust.codegen.smithy.generators.LibRsSection
import software.amazon.smithy.rust.codegen.smithy.generators.ProtocolConfig
import software.amazon.smithy.rust.codegen.smithy.letIf
import software.amazon.smithy.rust.codegen.smithy.protocols.ProtocolMap
import software.amazon.smithy.rust.codegen.smithy.protocols.RestXml
import software.amazon.smithy.rust.codegen.smithy.protocols.RestXmlFactory
import software.amazon.smithy.rustsdk.AwsRuntimeType
/**
* Top level decorator for S3
* */
class S3Decorator : RustCodegenDecorator {
override val name: String = "S3ExtendedError"
override val order: Byte = 0
private fun applies(serviceId: ShapeId) =
serviceId == ShapeId.from("com.amazonaws.s3#AmazonS3")
override fun protocols(serviceId: ShapeId, currentProtocols: ProtocolMap): ProtocolMap {
return currentProtocols.letIf(applies(serviceId)) {
it + mapOf(
RestXmlTrait.ID to RestXmlFactory { protocolConfig ->
S3(protocolConfig)
}
)
}
}
override fun libRsCustomizations(
protocolConfig: ProtocolConfig,
baseCustomizations: List<LibRsCustomization>
): List<LibRsCustomization> {
return baseCustomizations.letIf(applies(protocolConfig.serviceShape.id)) {
it + S3PubUse()
}
}
}
class S3(protocolConfig: ProtocolConfig) : RestXml(protocolConfig) {
private val runtimeConfig = protocolConfig.runtimeConfig
override fun parseGenericError(operationShape: OperationShape): RuntimeType {
return RuntimeType.forInlineFun("parse_generic_error", "xml_deser") {
it.rustBlockTemplate(
"pub fn parse_generic_error(response: &#{Response}<#{Bytes}>) -> Result<#{Error}, #{XmlError}>",
"Response" to RuntimeType.http.member("Response"),
"Bytes" to RuntimeType.Bytes,
"Error" to RuntimeType.GenericError(runtimeConfig),
"XmlError" to CargoDependency.smithyXml(runtimeConfig).asType().member("decode::XmlError")
) {
rustTemplate(
"""
let base_err = #{base_errors}::parse_generic_error(response.body().as_ref())?;
Ok(#{s3_errors}::parse_extended_error(base_err, &response))
""",
"base_errors" to restXmlErrors, "s3_errors" to AwsRuntimeType.S3Errors
)
}
}
}
}
class S3PubUse : LibRsCustomization() {
override fun section(section: LibRsSection): Writable = when (section) {
is LibRsSection.Body -> writable { rust("pub use #T::ErrorExt;", AwsRuntimeType.S3Errors) }
else -> emptySection
}
}

View File

@ -0,0 +1,14 @@
# This Cargo.toml is unused in generated code. It exists solely to enable these tests to compile in-situ
[package]
name = "s3-tests"
version = "0.1.0"
authors = ["Russell Cohen <rcoh@amazon.com>"]
edition = "2018"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
aws-sdk-s3 = { path = "../../build/aws-sdk/s3" }
smithy-http = { path = "../../build/aws-sdk/smithy-http" }
http = "0.2.3"
bytes = "1"

View File

@ -0,0 +1,4 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/

View File

@ -0,0 +1,38 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
use aws_sdk_s3::operation::GetObject;
use aws_sdk_s3::ErrorExt;
use bytes::Bytes;
use smithy_http::response::ParseHttpResponse;
#[test]
fn deserialize_extended_errors() {
let resp = http::Response::builder()
.header(
"x-amz-id-2",
"gyB+3jRPnrkN98ZajxHXr3u7EFM67bNgSAxexeEHndCX/7GRnfTXxReKUQF28IfP",
)
.header("x-amz-request-id", "3B3C7C725673C630")
.status(404)
.body(
r#"<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>NoSuchKey</Code>
<Message>The resource you requested does not exist</Message>
<Resource>/mybucket/myfoto.jpg</Resource>
<RequestId>4442587FB7D0A2F9</RequestId>
</Error>"#,
)
.unwrap();
let err = GetObject::new()
.parse_loaded(&resp.map(Bytes::from))
.expect_err("status was 404, this is an error");
assert_eq!(
err.meta().extended_request_id(),
Some("gyB+3jRPnrkN98ZajxHXr3u7EFM67bNgSAxexeEHndCX/7GRnfTXxReKUQF28IfP")
);
assert_eq!(err.meta().request_id(), Some("4442587FB7D0A2F9"));
}

View File

@ -72,18 +72,22 @@ class InlineDependency(
companion object {
fun forRustFile(
name: String,
baseDir: String,
vararg additionalDependencies: RustDependency
): InlineDependency {
val module = name
val filename = "$name.rs"
// The inline crate is loaded as a dependency on the runtime classpath
val rustFile = this::class.java.getResource("/inlineable/src/$filename")
check(rustFile != null) { "Rust file $filename was missing from the resource bundle!" }
val rustFile = this::class.java.getResource("/$baseDir/src/$filename")
check(rustFile != null) { "Rust file /$baseDir/src/$filename was missing from the resource bundle!" }
return InlineDependency(name, module, additionalDependencies.toList()) { writer ->
writer.raw(rustFile.readText())
}
}
private fun forRustFile(name: String, vararg additionalDependencies: RustDependency) =
forRustFile(name, "inlineable", *additionalDependencies)
fun awsJsonErrors(runtimeConfig: RuntimeConfig) =
forRustFile("aws_json_errors", CargoDependency.Http, CargoDependency.SmithyTypes(runtimeConfig))

View File

@ -70,7 +70,7 @@ fun <T : CodeWriter> T.rust(
@Language("Rust", prefix = "macro_rules! foo { () => {{ ", suffix = "}}}") contents: String,
vararg args: Any
) {
this.write(contents, *args)
this.write(contents.trim(), *args)
}
/**

View File

@ -174,12 +174,12 @@ data class RuntimeType(val name: String?, val dependency: RustDependency?, val n
fun awsJsonErrors(runtimeConfig: RuntimeConfig) =
forInlineDependency(InlineDependency.awsJsonErrors(runtimeConfig))
val DocJson = forInlineDependency(InlineDependency.docJson())
val DocJson by lazy { forInlineDependency(InlineDependency.docJson()) }
val InstantEpoch = forInlineDependency(InlineDependency.instantEpoch())
val InstantHttpDate = forInlineDependency(InlineDependency.instantHttpDate())
val Instant8601 = forInlineDependency(InlineDependency.instant8601())
val IdempotencyToken = forInlineDependency(InlineDependency.idempotencyToken())
val InstantEpoch by lazy { forInlineDependency(InlineDependency.instantEpoch()) }
val InstantHttpDate by lazy { forInlineDependency(InlineDependency.instantHttpDate()) }
val Instant8601 by lazy { forInlineDependency(InlineDependency.instant8601()) }
val IdempotencyToken by lazy { forInlineDependency(InlineDependency.idempotencyToken()) }
val Config = RuntimeType("config", null, "crate")
@ -207,7 +207,7 @@ data class RuntimeType(val name: String?, val dependency: RustDependency?, val n
val Bytes = RuntimeType("Bytes", dependency = CargoDependency.Bytes, namespace = "bytes")
fun BlobSerde(runtimeConfig: RuntimeConfig) = forInlineDependency(InlineDependency.blobSerde(runtimeConfig))
private fun forInlineDependency(inlineDependency: InlineDependency) =
fun forInlineDependency(inlineDependency: InlineDependency) =
RuntimeType(inlineDependency.name, inlineDependency, namespace = "crate")
fun forInlineFun(name: String, module: String, func: (RustWriter) -> Unit) = RuntimeType(

View File

@ -64,9 +64,6 @@ abstract class HttpProtocolGenerator(
operationShape: OperationShape,
customizations: List<OperationCustomization>
) {
/* if (operationShape.hasTrait<EndpointTrait>()) {
TODO("https://github.com/awslabs/smithy-rs/issues/197")
} */
val inputShape = operationShape.inputShape(model)
val sdkId =
protocolConfig.serviceShape.getTrait<ServiceTrait>()?.sdkId?.toLowerCase()?.replace(" ", "")
@ -162,7 +159,7 @@ abstract class HttpProtocolGenerator(
withBlock("Ok({", "})") {
features.forEach { it.section(OperationSection.MutateInput("self", "_config"))(this) }
rust("let request = self.request_builder_base()?;")
withBlock("let body = ", ";") {
withBlock("let body =", ";") {
body("self", shape)
}
rust("let request = Self::assemble(request, body);")

View File

@ -121,24 +121,25 @@ class CombinedErrorGenerator(
writer.rustTemplate(
"""
impl ${symbol.name} {
pub fn new(kind: ${symbol.name}Kind, meta: #{generic_error}) -> Self {
Self { kind, meta }
}
impl ${symbol.name} {
pub fn new(kind: ${symbol.name}Kind, meta: #{generic_error}) -> Self {
Self { kind, meta }
}
pub fn unhandled(err: impl Into<Box<dyn #{std_error} + Send + Sync + 'static>>) -> Self {
Self {
kind: ${symbol.name}Kind::Unhandled(err.into()),
meta: Default::default()
}
}
pub fn generic(err: #{generic_error}) -> Self {
Self {
meta: err.clone(),
kind: ${symbol.name}Kind::Unhandled(err.into()),
}
pub fn unhandled(err: impl Into<Box<dyn #{std_error} + Send + Sync + 'static>>) -> Self {
Self {
kind: ${symbol.name}Kind::Unhandled(err.into()),
meta: Default::default()
}
}
pub fn generic(err: #{generic_error}) -> Self {
Self {
meta: err.clone(),
kind: ${symbol.name}Kind::Unhandled(err.into()),
}
}
// Consider if this should actually be `Option<Cow<&str>>`. This would enable us to use display as implemented
// by std::Error to generate a message in that case.
@ -146,6 +147,10 @@ class CombinedErrorGenerator(
self.meta.message()
}
pub fn meta(&self) -> &#{generic_error} {
&self.meta
}
pub fn request_id(&self) -> Option<&str> {
self.meta.request_id()
}

View File

@ -88,7 +88,7 @@ class HttpTraitProtocolGenerator(
val bindings = httpIndex.getRequestBindings(operationShape).toList()
val payloadMemberName: String? =
bindings.firstOrNull { (_, binding) -> binding.location == HttpBinding.Location.PAYLOAD }?.first
if (payloadMemberName == null) {
return if (payloadMemberName == null) {
serializerGenerator.operationSerializer(operationShape)?.let { serializer ->
rust(
"#T(&self).map_err(|err|#T::SerializationError(err.into()))?",
@ -96,10 +96,10 @@ class HttpTraitProtocolGenerator(
runtimeConfig.operationBuildError()
)
} ?: rustTemplate("#{SdkBody}::from(\"\")", *codegenScope)
return BodyMetadata(takesOwnership = false)
BodyMetadata(takesOwnership = false)
} else {
val member = inputShape.expectMember(payloadMemberName)
return serializeViaPayload(member, serializerGenerator)
serializeViaPayload(member, serializerGenerator)
}
}
@ -470,8 +470,7 @@ class HttpTraitProtocolGenerator(
rust(
"""
#T(response.headers())
.map_err(|_|#T::unhandled("Failed to parse ${member.memberName} from header `${binding.locationName}"))?
""",
.map_err(|_|#T::unhandled("Failed to parse ${member.memberName} from header `${binding.locationName}"))?""",
fnName, errorSymbol
)
}

View File

@ -24,9 +24,12 @@ import software.amazon.smithy.rust.codegen.smithy.transformers.OperationNormaliz
import software.amazon.smithy.rust.codegen.smithy.transformers.RemoveEventStreamOperations
import software.amazon.smithy.rust.codegen.util.expectTrait
class RestXmlFactory : ProtocolGeneratorFactory<HttpTraitProtocolGenerator> {
override fun buildProtocolGenerator(protocolConfig: ProtocolConfig): HttpTraitProtocolGenerator {
return HttpTraitProtocolGenerator(protocolConfig, RestXml(protocolConfig))
class RestXmlFactory(private val generator: (ProtocolConfig) -> Protocol = { RestXml(it) }) :
ProtocolGeneratorFactory<HttpTraitProtocolGenerator> {
override fun buildProtocolGenerator(
protocolConfig: ProtocolConfig
): HttpTraitProtocolGenerator {
return HttpTraitProtocolGenerator(protocolConfig, generator(protocolConfig))
}
override fun transformModel(model: Model): Model {
@ -46,10 +49,10 @@ class RestXmlFactory : ProtocolGeneratorFactory<HttpTraitProtocolGenerator> {
}
}
class RestXml(private val protocolConfig: ProtocolConfig) : Protocol {
open class RestXml(private val protocolConfig: ProtocolConfig) : Protocol {
private val restXml = protocolConfig.serviceShape.expectTrait<RestXmlTrait>()
private val runtimeConfig = protocolConfig.runtimeConfig
private val restXmlErrors: RuntimeType = when (restXml.isNoErrorWrapping) {
protected val restXmlErrors: RuntimeType = when (restXml.isNoErrorWrapping) {
true -> RuntimeType.unwrappedXmlErrors(runtimeConfig)
false -> RuntimeType.wrappedXmlErrors(runtimeConfig)
}

View File

@ -227,7 +227,7 @@ class XmlBindingTraitParserGenerator(protocolConfig: ProtocolConfig, private val
private fun RustWriter.parseStructureInner(members: XmlMemberIndex, builder: String, outerCtx: Ctx) {
members.attributeMembers.forEach { member ->
val temp = safeName("attrib")
withBlock("let $temp = ", ";") {
withBlock("let $temp =", ";") {
parseAttributeMember(member, outerCtx)
}
rust("$builder.${symbolProvider.toMemberName(member)} = $temp;")
@ -240,7 +240,7 @@ class XmlBindingTraitParserGenerator(protocolConfig: ProtocolConfig, private val
members.dataMembers.forEach { member ->
case(member) {
val temp = safeName()
withBlock("let $temp = ", ";") {
withBlock("let $temp =", ";") {
parseMember(
member,
ctx.copy(accum = "$builder.${symbolProvider.toMemberName(member)}.take()")
@ -346,7 +346,7 @@ class XmlBindingTraitParserGenerator(protocolConfig: ProtocolConfig, private val
Some(_) => return Err(#{XmlError}::custom("mixed variants"))
})
"""
withBlock("let tmp = ", ";") {
withBlock("let tmp =", ";") {
parseMember(member, ctx.copy(accum = current))
}
rust("base = Some(#T::$variantName(tmp));", symbol)

View File

@ -153,6 +153,10 @@ pub mod error {
pub fn builder() -> Builder {
Builder::default()
}
pub fn into_builder(self) -> Builder {
Builder { inner: self }
}
}
impl ProvideErrorKind for Error {

View File

@ -23,3 +23,4 @@ include(":rust-runtime")
include(":aws:sdk-codegen")
include(":aws:sdk-codegen-test")
include(":aws:sdk")
include(":aws:rust-runtime")