From 1eeb930e7f069158e96009ff1738a5b10308342f Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 13 Nov 2023 17:03:25 -0800 Subject: [PATCH 01/13] Revise unhandled error variant according to RFC-39 --- aws/rust-runtime/aws-types/src/request_id.rs | 3 + .../smithy/rustsdk/BaseRequestIdDecorator.kt | 2 +- .../query-strings-are-correctly-encoded.rs | 29 ++++---- .../integration-tests/s3/tests/request_id.rs | 2 +- .../error/OperationErrorGenerator.kt | 68 ++++++++++++------- .../generators/error/ServiceErrorGenerator.kt | 67 +++++++++++++++--- .../rust/codegen/core/smithy/RuntimeType.kt | 1 - rust-runtime/aws-smithy-types/src/error.rs | 2 + .../aws-smithy-types/src/error/unhandled.rs | 4 ++ 9 files changed, 124 insertions(+), 54 deletions(-) diff --git a/aws/rust-runtime/aws-types/src/request_id.rs b/aws/rust-runtime/aws-types/src/request_id.rs index ed56612984..97dff4b110 100644 --- a/aws/rust-runtime/aws-types/src/request_id.rs +++ b/aws/rust-runtime/aws-types/src/request_id.rs @@ -11,6 +11,8 @@ use aws_smithy_runtime_api::http::Response; use aws_smithy_types::error::metadata::{ Builder as ErrorMetadataBuilder, ErrorMetadata, ProvideErrorMetadata, }; + +#[allow(deprecated)] use aws_smithy_types::error::Unhandled; /// Constant for the [`ErrorMetadata`] extra field that contains the request ID @@ -38,6 +40,7 @@ impl RequestId for ErrorMetadata { } } +#[allow(deprecated)] impl RequestId for Unhandled { fn request_id(&self) -> Option<&str> { self.meta().request_id() diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/BaseRequestIdDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/BaseRequestIdDecorator.kt index 5025ffedab..96d61b4f9f 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/BaseRequestIdDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/BaseRequestIdDecorator.kt @@ -146,7 +146,7 @@ abstract class BaseRequestIdDecorator : ClientCodegenDecorator { val sym = codegenContext.symbolProvider.toSymbol(error) rust("Self::${sym.name}(e) => #T,", wrapped) } - rust("Self::Unhandled(e) => e.$accessorFunctionName(),") + rust("Self::Unhandled(e) => e.meta.$accessorFunctionName(),") } } } diff --git a/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs b/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs index 902d07933b..2aff20b2cd 100644 --- a/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs +++ b/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs @@ -59,7 +59,6 @@ async fn test_s3_signer_query_string_with_all_valid_chars() { #[tokio::test] #[ignore] async fn test_query_strings_are_correctly_encoded() { - use aws_sdk_s3::operation::list_objects_v2::ListObjectsV2Error; use aws_smithy_runtime_api::client::result::SdkError; tracing_subscriber::fmt::init(); @@ -80,22 +79,18 @@ async fn test_query_strings_are_correctly_encoded() { .send() .await; if let Err(SdkError::ServiceError(context)) = res { - match context.err() { - ListObjectsV2Error::Unhandled(e) - if e.to_string().contains("SignatureDoesNotMatch") => - { - chars_that_break_signing.push(byte); - } - ListObjectsV2Error::Unhandled(e) if e.to_string().contains("InvalidUri") => { - chars_that_break_uri_parsing.push(byte); - } - ListObjectsV2Error::Unhandled(e) if e.to_string().contains("InvalidArgument") => { - chars_that_are_invalid_arguments.push(byte); - } - ListObjectsV2Error::Unhandled(e) if e.to_string().contains("InvalidToken") => { - panic!("refresh your credentials and run this test again"); - } - e => todo!("unexpected error: {:?}", e), + let err = context.err(); + let msg = err.to_string(); + if err.is_unhandled() && msg.contains("SignatureDoesNotMatch") { + chars_that_break_signing.push(byte); + } else if err.is_unhandled() && msg.to_string().contains("InvalidUri") { + chars_that_break_uri_parsing.push(byte); + } else if err.is_unhandled() && msg.to_string().contains("InvalidArgument") { + chars_that_are_invalid_arguments.push(byte); + } else if err.is_unhandled() && msg.to_string().contains("InvalidToken") { + panic!("refresh your credentials and run this test again"); + } else { + todo!("unexpected error: {:?}", err); } } } diff --git a/aws/sdk/integration-tests/s3/tests/request_id.rs b/aws/sdk/integration-tests/s3/tests/request_id.rs index 1ea7204426..046385abae 100644 --- a/aws/sdk/integration-tests/s3/tests/request_id.rs +++ b/aws/sdk/integration-tests/s3/tests/request_id.rs @@ -91,7 +91,7 @@ async fn get_request_id_from_unmodeled_error() { .expect_err("status 500") .into_service_error(); request.expect_request(); - assert!(matches!(err, GetObjectError::Unhandled(_))); + assert!(err.is_unhandled()); assert_eq!(Some("correct-request-id"), err.request_id()); assert_eq!(Some("correct-request-id"), err.meta().request_id()); assert_eq!( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt index dfa0c1b60e..1a63bb166f 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt @@ -28,7 +28,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.errorMetadata import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope -import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.unhandledError import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.customize.Section import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations @@ -86,6 +85,8 @@ class OperationErrorGenerator( rust( """ /// An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code). + ##[deprecated(note = "Don't directly match on `Unhandled`. Instead, match on `_` and use \ + the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.")] Unhandled(#T), """, unhandledError(runtimeConfig), @@ -114,15 +115,9 @@ class OperationErrorGenerator( "StdError" to RuntimeType.StdError, "ErrorMeta" to errorMetadata, ) { - rust( - """ - Self::Unhandled({ - let mut builder = #T::builder().source(source); - builder.set_meta(meta); - builder.build() - }) - """, - unhandledError(runtimeConfig), + rustTemplate( + """Self::Unhandled(#{Unhandled} { source, meta: meta.unwrap_or_default() })""", + "Unhandled" to unhandledError(runtimeConfig), ) } } @@ -131,8 +126,11 @@ class OperationErrorGenerator( private fun RustWriter.renderImplDisplay(errorSymbol: Symbol, errors: List) { rustBlock("impl #T for ${errorSymbol.name}", RuntimeType.Display) { rustBlock("fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result") { - delegateToVariants(errors) { - writable { rust("_inner.fmt(f)") } + delegateToVariants(errors) { variantMatch -> + when (variantMatch) { + is VariantMatch.Unhandled -> writable { rust("f.write_str(\"unhandled error\")") } + is VariantMatch.Modeled -> writable { rust("_inner.fmt(f)") } + } } } } @@ -142,8 +140,13 @@ class OperationErrorGenerator( val errorMetadataTrait = RuntimeType.provideErrorMetadataTrait(runtimeConfig) rustBlock("impl #T for ${errorSymbol.name}", errorMetadataTrait) { rustBlock("fn meta(&self) -> &#T", errorMetadata(runtimeConfig)) { - delegateToVariants(errors) { - writable { rust("#T::meta(_inner)", errorMetadataTrait) } + delegateToVariants(errors) { variantMatch -> + writable { + when (variantMatch) { + is VariantMatch.Unhandled -> rust("&_inner.meta") + is VariantMatch.Modeled -> rust("#T::meta(_inner)", errorMetadataTrait) + } + } } } } @@ -189,16 +192,16 @@ class OperationErrorGenerator( """ /// Creates the `${errorSymbol.name}::Unhandled` variant from any error type. pub fn unhandled(err: impl #{Into}<#{Box}>) -> Self { - Self::Unhandled(#{Unhandled}::builder().source(err).build()) + Self::Unhandled(#{Unhandled} { source: err.into(), meta: #{Default}::default() }) } - /// Creates the `${errorSymbol.name}::Unhandled` variant from a `#{error_metadata}`. - pub fn generic(err: #{error_metadata}) -> Self { - Self::Unhandled(#{Unhandled}::builder().source(err.clone()).meta(err).build()) + /// Creates the `${errorSymbol.name}::Unhandled` variant from an [`ErrorMetadata`](#{ErrorMetadata}). + pub fn generic(err: #{ErrorMetadata}) -> Self { + Self::Unhandled(#{Unhandled} { source: err.clone().into(), meta: err }) } """, *preludeScope, - "error_metadata" to errorMetadata, + "ErrorMetadata" to errorMetadata, "StdError" to RuntimeType.StdError, "Unhandled" to unhandledError(runtimeConfig), ) @@ -209,13 +212,15 @@ class OperationErrorGenerator( """, ) rustBlock("pub fn meta(&self) -> &#T", errorMetadata) { - rust("use #T;", RuntimeType.provideErrorMetadataTrait(runtimeConfig)) rustBlock("match self") { errors.forEach { error -> val errorVariantSymbol = symbolProvider.toSymbol(error) - rust("Self::${errorVariantSymbol.name}(e) => e.meta(),") + rustTemplate( + "Self::${errorVariantSymbol.name}(e) => #{ProvideErrorMetadata}::meta(e),", + "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(runtimeConfig), + ) } - rust("Self::Unhandled(e) => e.meta(),") + rust("Self::Unhandled(e) => &e.meta,") } } errors.forEach { error -> @@ -226,6 +231,14 @@ class OperationErrorGenerator( rust("matches!(self, Self::${errorVariantSymbol.name}(_))") } } + rust( + """ + /// True if this error is the `Unhandled` variant. + pub fn is_unhandled(&self) -> bool { + matches!(self, Self::Unhandled(_)) + } + """, + ) } } @@ -236,9 +249,14 @@ class OperationErrorGenerator( *preludeScope, "StdError" to RuntimeType.StdError, ) { - delegateToVariants(errors) { - writable { - rustTemplate("#{Some}(_inner)", *preludeScope) + delegateToVariants(errors) { variantMatch -> + when (variantMatch) { + is VariantMatch.Unhandled -> writable { + rustTemplate("#{Some}(&*_inner.source)", *preludeScope) + } + is VariantMatch.Modeled -> writable { + rustTemplate("#{Some}(_inner)", *preludeScope) + } } } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt index f6fc9ed166..6f8cf969f7 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt @@ -9,6 +9,7 @@ import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.shapes.ShapeId import software.amazon.smithy.model.shapes.StructureShape +import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata import software.amazon.smithy.rust.codegen.core.rustlang.RustModule @@ -24,9 +25,9 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope -import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.unhandledError import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations import software.amazon.smithy.rust.codegen.core.smithy.generators.operationBuildError @@ -67,6 +68,7 @@ class ServiceErrorGenerator( fun render(crate: RustCrate) { crate.withModule(RustModule.private("error_meta")) { renderDefinition() + renderImpl() renderImplDisplay() renderImplFromBuildError() renderImplProvideErrorMetadata() @@ -91,7 +93,7 @@ class ServiceErrorGenerator( allErrors.forEach { rust("Error::${symbolProvider.toSymbol(it).name}(inner) => inner.source(),") } - rust("Error::Unhandled(inner) => inner.source()") + rustTemplate("Error::Unhandled(inner) => #{Some}(&*inner.source)", *preludeScope) } } } @@ -107,7 +109,7 @@ class ServiceErrorGenerator( allErrors.forEach { rust("Error::${symbolProvider.toSymbol(it).name}(inner) => inner.fmt(f),") } - rust("Error::Unhandled(inner) => inner.fmt(f)") + rust("Error::Unhandled(_inner) => f.write_str(\"unhandled error\")") } } } @@ -118,11 +120,12 @@ class ServiceErrorGenerator( """ impl From<#{BuildError}> for Error { fn from(value: #{BuildError}) -> Self { - Error::Unhandled(#{Unhandled}::builder().source(value).build()) + Error::Unhandled(#{Unhandled} { source: value.into(), meta: #{Default}::default() }) } } """, + *preludeScope, "BuildError" to codegenContext.runtimeConfig.operationBuildError(), "Unhandled" to unhandledError(codegenContext.runtimeConfig), ) @@ -146,10 +149,10 @@ class ServiceErrorGenerator( rustTemplate( """ _ => Error::Unhandled( - #{Unhandled}::builder() - .meta(#{ProvideErrorMetadata}::meta(&err).clone()) - .source(err) - .build() + #{Unhandled} { + meta: #{ProvideErrorMetadata}::meta(&err).clone(), + source: err.into(), + } ), """, "Unhandled" to unhandledError(codegenContext.runtimeConfig), @@ -187,7 +190,7 @@ class ServiceErrorGenerator( fn meta(&self) -> &#{ErrorMetadata} { match self { #{matchers} - Self::Unhandled(inner) => inner.meta(), + Self::Unhandled(inner) => &inner.meta, } } } @@ -220,7 +223,53 @@ class ServiceErrorGenerator( rust("${sym.name}(#T),", sym) } docs("An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code).") + rust( + "##[deprecated(note = \"Don't directly match on `Unhandled`. Instead, match on `_` and use " + + "the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.\")]", + ) rust("Unhandled(#T)", unhandledError(codegenContext.runtimeConfig)) } } + + private fun RustWriter.renderImpl() { + rustBlock("impl Error") { + rust( + """ + /// True if this error is the `Unhandled` variant. + pub fn is_unhandled(&self) -> bool { + matches!(self, Self::Unhandled(_)) + } + """, + ) + } + } +} + +fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( + "Unhandled", + // Place in a sealed module so that it can't be referenced at all + RustModule.pubCrate("sealed_unhandled", ClientRustModule.Error), +) { + rustTemplate( + """ + /// This struct is not intended to be used. + /// + /// This struct holds information about an unhandled error, + /// but that information should be obtained by using the + /// [`ProvideErrorMetadata`](#{ProvideErrorMetadata}) trait + /// on the error type. + /// + /// This struct intentionally doesn't yield any useful information itself. + ##[deprecated(note = "Don't directly match on `Unhandled`. Instead, match on `_` and use \ + the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.")] + ##[derive(Debug)] + pub struct Unhandled { + pub(crate) source: #{BoxError}, + pub(crate) meta: #{ErrorMetadata}, + } + """, + "BoxError" to RuntimeType.smithyRuntimeApi(rc).resolve("box_error::BoxError"), + "ErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ErrorMetadata"), + "ProvideErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ProvideErrorMetadata"), + ) } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt index 5e552d7cd5..3a12fbec76 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt @@ -425,7 +425,6 @@ data class RuntimeType(val path: String, val dependency: RustDependency? = null) fun provideErrorMetadataTrait(runtimeConfig: RuntimeConfig) = smithyTypes(runtimeConfig).resolve("error::metadata::ProvideErrorMetadata") - fun unhandledError(runtimeConfig: RuntimeConfig) = smithyTypes(runtimeConfig).resolve("error::Unhandled") fun jsonErrors(runtimeConfig: RuntimeConfig) = forInlineDependency(InlineDependency.jsonErrors(runtimeConfig)) fun awsQueryCompatibleErrors(runtimeConfig: RuntimeConfig) = forInlineDependency(InlineDependency.awsQueryCompatibleErrors(runtimeConfig)) diff --git a/rust-runtime/aws-smithy-types/src/error.rs b/rust-runtime/aws-smithy-types/src/error.rs index 96a48441e0..28838e4b76 100644 --- a/rust-runtime/aws-smithy-types/src/error.rs +++ b/rust-runtime/aws-smithy-types/src/error.rs @@ -13,6 +13,8 @@ pub mod operation; mod unhandled; pub use metadata::ErrorMetadata; + +#[allow(deprecated)] pub use unhandled::Unhandled; #[derive(Debug)] diff --git a/rust-runtime/aws-smithy-types/src/error/unhandled.rs b/rust-runtime/aws-smithy-types/src/error/unhandled.rs index 2397d700ff..f0cce422fc 100644 --- a/rust-runtime/aws-smithy-types/src/error/unhandled.rs +++ b/rust-runtime/aws-smithy-types/src/error/unhandled.rs @@ -3,11 +3,14 @@ * SPDX-License-Identifier: Apache-2.0 */ +#![allow(deprecated)] + //! Unhandled error type. use crate::error::{metadata::ProvideErrorMetadata, ErrorMetadata}; use std::error::Error as StdError; +#[deprecated(note = "The `Unhandled` type is no longer used by errors.")] /// Builder for [`Unhandled`] #[derive(Default, Debug)] pub struct Builder { @@ -58,6 +61,7 @@ impl Builder { /// [`DisplayErrorContext`](crate::error::display::DisplayErrorContext), use another /// error reporter library that visits the error's cause/source chain, or call /// [`Error::source`](std::error::Error::source) for more details about the underlying cause. +#[deprecated(note = "This type is no longer used by errors.")] #[derive(Debug)] pub struct Unhandled { source: Box, From 2d276b0fa868a14bc9dbffd4eae5cd23c322481b Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 13 Nov 2023 17:28:46 -0800 Subject: [PATCH 02/13] Add the deprecation pointer to the enum unknown variant --- .../smithy/generators/ClientEnumGenerator.kt | 18 ++++++++++++++---- .../generators/ClientEnumGeneratorTest.kt | 10 ++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt index db298141f0..2472735b8a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt @@ -10,6 +10,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.core.rustlang.RustModule import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.rustlang.Visibility import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.rust @@ -81,6 +82,10 @@ data class InfallibleEnumType( override fun additionalEnumMembers(context: EnumGeneratorContext): Writable = writable { docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") + rust( + """##[deprecated(note = "Don't directly match on `$UnknownVariant`. Instead, match on `_` and use \ + the `${context.enumName}::as_str()` method to retrieve information about the unknown enum value.")]""", + ) rust("$UnknownVariant(#T)", unknownVariantValue(context)) } @@ -93,10 +98,9 @@ data class InfallibleEnumType( docs( """ Opaque struct used as inner data for the `Unknown` variant defined in enums in - the crate + the crate. - While this is not intended to be used directly, it is marked as `pub` because it is - part of the enums that are public interface. + This is not intended to be used directly. """.trimIndent(), ) context.enumMeta.render(this) @@ -174,5 +178,11 @@ class ClientEnumGenerator(codegenContext: ClientCodegenContext, shape: StringSha codegenContext.model, codegenContext.symbolProvider, shape, - InfallibleEnumType(ClientRustModule.primitives), + InfallibleEnumType( + RustModule.new( + "sealed_enum_unknown", + visibility = Visibility.PUBCRATE, + parent = ClientRustModule.primitives, + ), + ), ) diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGeneratorTest.kt index 4354033485..9c0817e5c5 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGeneratorTest.kt @@ -119,7 +119,10 @@ class ClientEnumGeneratorTest { """ assert_eq!(SomeEnum::from("Unknown"), SomeEnum::UnknownValue); assert_eq!(SomeEnum::from("UnknownValue"), SomeEnum::UnknownValue_); - assert_eq!(SomeEnum::from("SomethingNew"), SomeEnum::Unknown(crate::primitives::UnknownVariantValue("SomethingNew".to_owned()))); + assert_eq!( + SomeEnum::from("SomethingNew"), + SomeEnum::Unknown(crate::primitives::sealed_enum_unknown::UnknownVariantValue("SomethingNew".to_owned())) + ); """, ) } @@ -150,7 +153,10 @@ class ClientEnumGeneratorTest { assert_eq!(instance.as_str(), "t2.micro"); assert_eq!(InstanceType::from("t2.nano"), InstanceType::T2Nano); // round trip unknown variants: - assert_eq!(InstanceType::from("other"), InstanceType::Unknown(crate::primitives::UnknownVariantValue("other".to_owned()))); + assert_eq!( + InstanceType::from("other"), + InstanceType::Unknown(crate::primitives::sealed_enum_unknown::UnknownVariantValue("other".to_owned())) + ); assert_eq!(InstanceType::from("other").as_str(), "other"); """, ) From 7c406996dff7c1f4fbb9c52dbbf65026eb101881 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 10:59:02 -0800 Subject: [PATCH 03/13] Fix several CI failures --- aws/rust-runtime/aws-inlineable/src/s3_request_id.rs | 2 ++ aws/sdk/integration-tests/lambda/tests/request_id.rs | 1 + .../eventstream/ClientEventStreamUnmarshallerGeneratorTest.kt | 2 +- .../pokemon-service-client-usage/examples/handling-errors.rs | 4 ---- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/aws/rust-runtime/aws-inlineable/src/s3_request_id.rs b/aws/rust-runtime/aws-inlineable/src/s3_request_id.rs index 20962244b0..53b23c3cf2 100644 --- a/aws/rust-runtime/aws-inlineable/src/s3_request_id.rs +++ b/aws/rust-runtime/aws-inlineable/src/s3_request_id.rs @@ -8,6 +8,7 @@ use aws_smithy_runtime_api::http::{Headers, Response}; use aws_smithy_types::error::metadata::{ Builder as ErrorMetadataBuilder, ErrorMetadata, ProvideErrorMetadata, }; +#[allow(deprecated)] use aws_smithy_types::error::Unhandled; const EXTENDED_REQUEST_ID: &str = "s3_extended_request_id"; @@ -36,6 +37,7 @@ impl RequestIdExt for ErrorMetadata { } } +#[allow(deprecated)] impl RequestIdExt for Unhandled { fn extended_request_id(&self) -> Option<&str> { self.meta().extended_request_id() diff --git a/aws/sdk/integration-tests/lambda/tests/request_id.rs b/aws/sdk/integration-tests/lambda/tests/request_id.rs index 6a6e1384ae..4517f05bc9 100644 --- a/aws/sdk/integration-tests/lambda/tests/request_id.rs +++ b/aws/sdk/integration-tests/lambda/tests/request_id.rs @@ -9,6 +9,7 @@ use aws_sdk_lambda::operation::RequestId; use aws_sdk_lambda::{Client, Config}; use aws_smithy_runtime::client::http::test_util::infallible_client_fn; +#[allow(deprecated)] async fn run_test( response: impl Fn() -> http::Response<&'static str> + Send + Sync + 'static, expect_error: bool, diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/eventstream/ClientEventStreamUnmarshallerGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/eventstream/ClientEventStreamUnmarshallerGeneratorTest.kt index d0118d1f84..92d0b3663c 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/eventstream/ClientEventStreamUnmarshallerGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/eventstream/ClientEventStreamUnmarshallerGeneratorTest.kt @@ -51,7 +51,7 @@ class ClientEventStreamUnmarshallerGeneratorTest { let result = $generator::new().unmarshall(&message); assert!(result.is_ok(), "expected ok, got: {:?}", result); match expect_error(result.unwrap()) { - TestStreamError::Unhandled(err) => { + err @ TestStreamError::Unhandled(_) => { let message = format!("{}", crate::error::DisplayErrorContext(&err)); let expected = "message: \"unmodeled error\""; assert!(message.contains(expected), "Expected '{message}' to contain '{expected}'"); diff --git a/examples/pokemon-service-client-usage/examples/handling-errors.rs b/examples/pokemon-service-client-usage/examples/handling-errors.rs index af1c230cdc..1c1da7283c 100644 --- a/examples/pokemon-service-client-usage/examples/handling-errors.rs +++ b/examples/pokemon-service-client-usage/examples/handling-errors.rs @@ -77,10 +77,6 @@ async fn main() { GetStorageError::ValidationError(ve) => { tracing::error!(error = %ve, "A required field has not been set."); } - // An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code). - GetStorageError::Unhandled(uh) => { - tracing::error!(error = %uh, "An unhandled error has occurred.") - } // The SdkError is marked as `#[non_exhaustive]`. Therefore, a catch-all pattern is required to handle // potential future variants introduced in SdkError. _ => { From 5b1952df1aa45e3e44a7083581fae2db1d775fe5 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 13:14:50 -0800 Subject: [PATCH 04/13] Remove `is_unhandled()` --- .../tests/query-strings-are-correctly-encoded.rs | 11 +++++++---- aws/sdk/integration-tests/s3/tests/request_id.rs | 3 ++- .../generators/error/OperationErrorGenerator.kt | 8 -------- .../generators/error/ServiceErrorGenerator.kt | 14 -------------- 4 files changed, 9 insertions(+), 27 deletions(-) diff --git a/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs b/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs index 2aff20b2cd..563ba7076d 100644 --- a/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs +++ b/aws/sdk/integration-tests/s3/tests/query-strings-are-correctly-encoded.rs @@ -7,6 +7,7 @@ use aws_credential_types::provider::SharedCredentialsProvider; use aws_sdk_s3::config::{Credentials, Region}; +use aws_sdk_s3::operation::list_objects_v2::ListObjectsV2Error; use aws_sdk_s3::{Client, Config}; use aws_smithy_runtime::client::http::test_util::capture_request; @@ -58,6 +59,7 @@ async fn test_s3_signer_query_string_with_all_valid_chars() { // test must be run against an actual bucket so we `ignore` it unless the runner specifically requests it #[tokio::test] #[ignore] +#[allow(deprecated)] async fn test_query_strings_are_correctly_encoded() { use aws_smithy_runtime_api::client::result::SdkError; @@ -81,13 +83,14 @@ async fn test_query_strings_are_correctly_encoded() { if let Err(SdkError::ServiceError(context)) = res { let err = context.err(); let msg = err.to_string(); - if err.is_unhandled() && msg.contains("SignatureDoesNotMatch") { + let unhandled = matches!(err, ListObjectsV2Error::Unhandled(_)); + if unhandled && msg.contains("SignatureDoesNotMatch") { chars_that_break_signing.push(byte); - } else if err.is_unhandled() && msg.to_string().contains("InvalidUri") { + } else if unhandled && msg.to_string().contains("InvalidUri") { chars_that_break_uri_parsing.push(byte); - } else if err.is_unhandled() && msg.to_string().contains("InvalidArgument") { + } else if unhandled && msg.to_string().contains("InvalidArgument") { chars_that_are_invalid_arguments.push(byte); - } else if err.is_unhandled() && msg.to_string().contains("InvalidToken") { + } else if unhandled && msg.to_string().contains("InvalidToken") { panic!("refresh your credentials and run this test again"); } else { todo!("unexpected error: {:?}", err); diff --git a/aws/sdk/integration-tests/s3/tests/request_id.rs b/aws/sdk/integration-tests/s3/tests/request_id.rs index 046385abae..886b2d5baf 100644 --- a/aws/sdk/integration-tests/s3/tests/request_id.rs +++ b/aws/sdk/integration-tests/s3/tests/request_id.rs @@ -59,6 +59,7 @@ async fn get_request_id_from_modeled_error() { } #[tokio::test] +#[allow(deprecated)] async fn get_request_id_from_unmodeled_error() { let (http_client, request) = capture_request(Some( http::Response::builder() @@ -91,7 +92,7 @@ async fn get_request_id_from_unmodeled_error() { .expect_err("status 500") .into_service_error(); request.expect_request(); - assert!(err.is_unhandled()); + assert!(matches!(err, GetObjectError::Unhandled(_))); assert_eq!(Some("correct-request-id"), err.request_id()); assert_eq!(Some("correct-request-id"), err.meta().request_id()); assert_eq!( diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt index 1a63bb166f..dc20c9523c 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt @@ -231,14 +231,6 @@ class OperationErrorGenerator( rust("matches!(self, Self::${errorVariantSymbol.name}(_))") } } - rust( - """ - /// True if this error is the `Unhandled` variant. - pub fn is_unhandled(&self) -> bool { - matches!(self, Self::Unhandled(_)) - } - """, - ) } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt index 6f8cf969f7..4b6501fa8b 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt @@ -68,7 +68,6 @@ class ServiceErrorGenerator( fun render(crate: RustCrate) { crate.withModule(RustModule.private("error_meta")) { renderDefinition() - renderImpl() renderImplDisplay() renderImplFromBuildError() renderImplProvideErrorMetadata() @@ -230,19 +229,6 @@ class ServiceErrorGenerator( rust("Unhandled(#T)", unhandledError(codegenContext.runtimeConfig)) } } - - private fun RustWriter.renderImpl() { - rustBlock("impl Error") { - rust( - """ - /// True if this error is the `Unhandled` variant. - pub fn is_unhandled(&self) -> bool { - matches!(self, Self::Unhandled(_)) - } - """, - ) - } - } } fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( From b308867c196a4f3cb2da7eb0195a60699bcf1b03 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 13:46:55 -0800 Subject: [PATCH 05/13] Incorporate feedback --- .../smithy/generators/ClientEnumGenerator.kt | 3 +- .../error/OperationErrorGenerator.kt | 23 ++++++++++---- .../generators/error/ServiceErrorGenerator.kt | 31 ++++++++++++++----- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt index 2472735b8a..7d3d795175 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt @@ -83,8 +83,7 @@ data class InfallibleEnumType( override fun additionalEnumMembers(context: EnumGeneratorContext): Writable = writable { docs("`$UnknownVariant` contains new variants that have been added since this code was generated.") rust( - """##[deprecated(note = "Don't directly match on `$UnknownVariant`. Instead, match on `_` and use \ - the `${context.enumName}::as_str()` method to retrieve information about the unknown enum value.")]""", + """##[deprecated(note = "Don't directly match on `$UnknownVariant`. See the docs on this enum for the correct way to handle unknown variants.")]""", ) rust("$UnknownVariant(#T)", unknownVariantValue(context)) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt index dc20c9523c..6358d267e3 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt @@ -82,14 +82,14 @@ class OperationErrorGenerator( val errorVariantSymbol = symbolProvider.toSymbol(errorVariant) write("${errorVariantSymbol.name}(#T),", errorVariantSymbol) } - rust( + rustTemplate( """ /// An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code). - ##[deprecated(note = "Don't directly match on `Unhandled`. Instead, match on `_` and use \ - the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.")] - Unhandled(#T), + #{deprecation} + Unhandled(#{Unhandled}), """, - unhandledError(runtimeConfig), + "deprecation" to writable { renderUnhandledErrorDeprecation() }, + "Unhandled" to unhandledError(runtimeConfig), ) } @@ -128,7 +128,18 @@ class OperationErrorGenerator( rustBlock("fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result") { delegateToVariants(errors) { variantMatch -> when (variantMatch) { - is VariantMatch.Unhandled -> writable { rust("f.write_str(\"unhandled error\")") } + is VariantMatch.Unhandled -> writable { + rustTemplate( + """ + if let Some(code) = #{ProvideErrorMetadata}::code(self) { + write!(f, "unhandled error ({code})") + } else { + f.write_str("unhandled error") + } + """, + "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(runtimeConfig), + ) + } is VariantMatch.Modeled -> writable { rust("_inner.fmt(f)") } } } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt index 4b6501fa8b..239363c2ce 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt @@ -108,7 +108,16 @@ class ServiceErrorGenerator( allErrors.forEach { rust("Error::${symbolProvider.toSymbol(it).name}(inner) => inner.fmt(f),") } - rust("Error::Unhandled(_inner) => f.write_str(\"unhandled error\")") + rustTemplate( + """ + Error::Unhandled(_) => if let Some(code) = #{ProvideErrorMetadata}::code(self) { + write!(f, "unhandled error ({code})") + } else { + f.write_str("unhandled error") + } + """, + "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(codegenContext.runtimeConfig), + ) } } } @@ -222,10 +231,7 @@ class ServiceErrorGenerator( rust("${sym.name}(#T),", sym) } docs("An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code).") - rust( - "##[deprecated(note = \"Don't directly match on `Unhandled`. Instead, match on `_` and use " + - "the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.\")]", - ) + renderUnhandledErrorDeprecation() rust("Unhandled(#T)", unhandledError(codegenContext.runtimeConfig)) } } @@ -246,8 +252,7 @@ fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( /// on the error type. /// /// This struct intentionally doesn't yield any useful information itself. - ##[deprecated(note = "Don't directly match on `Unhandled`. Instead, match on `_` and use \ - the `ProvideErrorMetadata` trait to retrieve information about the unhandled error.")] + #{deprecation} ##[derive(Debug)] pub struct Unhandled { pub(crate) source: #{BoxError}, @@ -255,7 +260,19 @@ fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( } """, "BoxError" to RuntimeType.smithyRuntimeApi(rc).resolve("box_error::BoxError"), + "deprecation" to writable { renderUnhandledErrorDeprecation() }, "ErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ErrorMetadata"), "ProvideErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ProvideErrorMetadata"), ) } + +fun RustWriter.renderUnhandledErrorDeprecation() { + val message = """ + Don't directly match on `Unhandled`. Instead, match using the catch all matcher (e.g., `_`), + give it a name like `err`, and use the `ProvideErrorMetadata` trait to retrieve the error + code and message. For example: `err => println!("code: {}, message: {}", err.code(), err.message())` + """.trimIndent() + // `.dq()` doesn't quite do what we want here since we actually want a Rust multi-line string + val messageEscaped = message.replace("\"", "\\\"").replace("\n", " \\\n") + rust("""##[deprecated(note = "$messageEscaped")]""") +} From fcc8133a4f33babfa436d7595c0d48c2f4bada55 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 14:16:49 -0800 Subject: [PATCH 06/13] Update changelog --- CHANGELOG.next.toml | 58 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 0113a7eba7..32dd666362 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -82,3 +82,61 @@ message = "Remove deprecated error kind type aliases." references = ["smithy-rs#3189"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } author = "jdisanti" + +[[aws-sdk-rust]] +message = """ +Unhandled errors have been made opaque to ensure code is written in a future-proof manner. Where previously, you +might have: +```rust +match service_error.err() { + GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } + GetStorageError::Unhandled(unhandled) if unhandled.code() == "SomeUnmodeledErrorCode" { + // unhandled error handling + } + _ => { /* ... */ } +} +``` +It should now look as follows: +```rust +match service_error.err() { + GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } + err if err.code() == "SomeUnmodeledErrorCode" { + // unhandled error handling + } + _ => { /* ... */ } +} +``` +The `Unhandled` variant should never be referenced directly. +""" +references = ["smithy-rs#3191"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "jdisanti" + +[[smithy-rs]] +message = """ +Unhandled errors have been made opaque to ensure code is written in a future-proof manner. Where previously, you +might have: +```rust +match service_error.err() { + GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } + GetStorageError::Unhandled(unhandled) if unhandled.code() == "SomeUnmodeledErrorCode" { + // unhandled error handling + } + _ => { /* ... */ } +} +``` +It should now look as follows: +```rust +match service_error.err() { + GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } + err if err.code() == "SomeUnmodeledErrorCode" { + // unhandled error handling + } + _ => { /* ... */ } +} +``` +The `Unhandled` variant should never be referenced directly. +""" +references = ["smithy-rs#3191"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } +author = "jdisanti" From 3391a9b9d47adc01025828a3a2ca239901d6fc17 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 14:18:44 -0800 Subject: [PATCH 07/13] Fix name collision --- .../client/smithy/generators/error/OperationErrorGenerator.kt | 3 ++- .../client/smithy/generators/error/ServiceErrorGenerator.kt | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt index 6358d267e3..9cb3082c20 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt @@ -131,12 +131,13 @@ class OperationErrorGenerator( is VariantMatch.Unhandled -> writable { rustTemplate( """ - if let Some(code) = #{ProvideErrorMetadata}::code(self) { + if let #{Some}(code) = #{ProvideErrorMetadata}::code(self) { write!(f, "unhandled error ({code})") } else { f.write_str("unhandled error") } """, + *preludeScope, "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(runtimeConfig), ) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt index 239363c2ce..c4bb7ae114 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt @@ -110,12 +110,13 @@ class ServiceErrorGenerator( } rustTemplate( """ - Error::Unhandled(_) => if let Some(code) = #{ProvideErrorMetadata}::code(self) { + Error::Unhandled(_) => if let #{Some}(code) = #{ProvideErrorMetadata}::code(self) { write!(f, "unhandled error ({code})") } else { f.write_str("unhandled error") } """, + *preludeScope, "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(codegenContext.runtimeConfig), ) } From 3fdb09b9f3fdab34a77c1ed821b7aa5d0430c586 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 18:04:32 -0800 Subject: [PATCH 08/13] Add `try_parse` method to client enums --- .../smithy/generators/ClientEnumGenerator.kt | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt index 7d3d795175..2fb314e824 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt @@ -76,6 +76,27 @@ data class InfallibleEnumType( ) } + override fun additionalEnumImpls(context: EnumGeneratorContext): Writable = writable { + rustTemplate( + """ + impl ${context.enumName} { + /// Parses the enum value while disallowing unknown variants. + /// + /// Unknown variants will result in an error. + pub fn try_parse(value: &str) -> #{Result} { + match Self::from(value) { + ##[allow(deprecated)] + Self::Unknown(_) => #{Err}(#{UnknownVariantError}::new(value)), + known => Ok(known), + } + } + } + """, + *preludeScope, + "UnknownVariantError" to unknownVariantError(), + ) + } + override fun additionalDocs(context: EnumGeneratorContext): Writable = writable { renderForwardCompatibilityNote(context.enumName, context.sortedMembers, UnknownVariant, UnknownVariantValue) } @@ -185,3 +206,27 @@ class ClientEnumGenerator(codegenContext: ClientCodegenContext, shape: StringSha ), ), ) + +private fun unknownVariantError(): RuntimeType = RuntimeType.forInlineFun("UnknownVariantError", ClientRustModule.Error) { + rustTemplate( + """ + /// The given enum value failed to parse since it is not a known value. + ##[derive(Debug)] + pub struct UnknownVariantError { + value: #{String}, + } + impl UnknownVariantError { + pub(crate) fn new(value: impl #{Into}<#{String}>) -> Self { + Self { value: value.into() } + } + } + impl ::std::fmt::Display for UnknownVariantError { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> #{Result}<(), ::std::fmt::Error> { + write!(f, "unknown enum variant: '{}'", self.value) + } + } + impl ::std::error::Error for UnknownVariantError {} + """, + *preludeScope, + ) +} From bf24f21d8fdda93d428cad90c1daf955525dc72d Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 18:08:33 -0800 Subject: [PATCH 09/13] Use `DisplayErrorContext` in example --- .../examples/handling-errors.rs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/examples/pokemon-service-client-usage/examples/handling-errors.rs b/examples/pokemon-service-client-usage/examples/handling-errors.rs index 1c1da7283c..dfcebda610 100644 --- a/examples/pokemon-service-client-usage/examples/handling-errors.rs +++ b/examples/pokemon-service-client-usage/examples/handling-errors.rs @@ -2,20 +2,19 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -/// Copyright © 2023, Amazon, LLC. -/// -/// This example demonstrates how to handle service generated errors. -/// -/// The example assumes that the Pokémon service is running on the localhost on TCP port 13734. -/// Refer to the [README.md](https://github.com/smithy-lang/smithy-rs/tree/main/examples/pokemon-service-client-usage/README.md) -/// file for instructions on how to launch the service locally. -/// -/// The example can be run using `cargo run --example handling-errors`. -/// -use pokemon_service_client::{error::SdkError, operation::get_storage::GetStorageError}; -use pokemon_service_client_usage::{setup_tracing_subscriber, POKEMON_SERVICE_URL}; +//! This example demonstrates how to handle service generated errors. +//! +//! The example assumes that the Pokémon service is running on the localhost on TCP port 13734. +//! Refer to the [README.md](https://github.com/smithy-lang/smithy-rs/tree/main/examples/pokemon-service-client-usage/README.md) +//! file for instructions on how to launch the service locally. +//! +//! The example can be run using `cargo run --example handling-errors`. + +use pokemon_service_client::error::DisplayErrorContext; use pokemon_service_client::Client as PokemonClient; +use pokemon_service_client::{error::SdkError, operation::get_storage::GetStorageError}; +use pokemon_service_client_usage::{setup_tracing_subscriber, POKEMON_SERVICE_URL}; /// Creates a new `smithy-rs` client that is configured to communicate with a locally running Pokémon service on TCP port 13734. /// @@ -80,7 +79,7 @@ async fn main() { // The SdkError is marked as `#[non_exhaustive]`. Therefore, a catch-all pattern is required to handle // potential future variants introduced in SdkError. _ => { - tracing::error!(error = %se.err(), "Some other error has occurred on the server") + tracing::error!(error = %DisplayErrorContext(se.err()), "Some other error has occurred on the server") } } } From abbb142741bbd0a8f6e5c54a47795d9795766c5a Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 18:10:32 -0800 Subject: [PATCH 10/13] Fix changelog code --- CHANGELOG.next.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 3a289ccc27..739891c16f 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -127,7 +127,7 @@ might have: ```rust match service_error.err() { GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } - GetStorageError::Unhandled(unhandled) if unhandled.code() == "SomeUnmodeledErrorCode" { + GetStorageError::Unhandled(unhandled) if unhandled.code() == Some("SomeUnmodeledErrorCode") { // unhandled error handling } _ => { /* ... */ } @@ -137,7 +137,7 @@ It should now look as follows: ```rust match service_error.err() { GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } - err if err.code() == "SomeUnmodeledErrorCode" { + err if err.code() == Some("SomeUnmodeledErrorCode") { // unhandled error handling } _ => { /* ... */ } @@ -156,7 +156,7 @@ might have: ```rust match service_error.err() { GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } - GetStorageError::Unhandled(unhandled) if unhandled.code() == "SomeUnmodeledErrorCode" { + GetStorageError::Unhandled(unhandled) if unhandled.code() == Some("SomeUnmodeledErrorCode") { // unhandled error handling } _ => { /* ... */ } @@ -166,7 +166,7 @@ It should now look as follows: ```rust match service_error.err() { GetStorageError::StorageAccessNotAuthorized(_) => { /* ... */ } - err if err.code() == "SomeUnmodeledErrorCode" { + err if err.code() == Some("SomeUnmodeledErrorCode") { // unhandled error handling } _ => { /* ... */ } From 34b3dea55a461cafbe24fb36884cd4c029d35b7c Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 18:19:23 -0800 Subject: [PATCH 11/13] Fix visibility of `StatusCode` --- rust-runtime/aws-smithy-runtime-api/src/http.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-runtime-api/src/http.rs b/rust-runtime/aws-smithy-runtime-api/src/http.rs index a133e23db7..f295b8ac36 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/http.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/http.rs @@ -13,4 +13,4 @@ mod response; pub use error::HttpError; pub use headers::{HeaderValue, Headers, HeadersIter}; pub use request::{Request, RequestParts}; -pub use response::Response; +pub use response::{Response, StatusCode}; From 759c58d90d1cc98b168aa2e3e2aba736259d3351 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 14 Nov 2023 19:08:49 -0800 Subject: [PATCH 12/13] Fix more CI issues --- .../smithy/generators/ClientEnumGenerator.kt | 35 ++++++++++--------- .../external-types.toml | 1 + 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt index 2fb314e824..c665144c23 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ClientEnumGenerator.kt @@ -77,24 +77,27 @@ data class InfallibleEnumType( } override fun additionalEnumImpls(context: EnumGeneratorContext): Writable = writable { - rustTemplate( - """ - impl ${context.enumName} { - /// Parses the enum value while disallowing unknown variants. - /// - /// Unknown variants will result in an error. - pub fn try_parse(value: &str) -> #{Result} { - match Self::from(value) { - ##[allow(deprecated)] - Self::Unknown(_) => #{Err}(#{UnknownVariantError}::new(value)), - known => Ok(known), + // `try_parse` isn't needed for unnamed enums + if (context.enumTrait.hasNames()) { + rustTemplate( + """ + impl ${context.enumName} { + /// Parses the enum value while disallowing unknown variants. + /// + /// Unknown variants will result in an error. + pub fn try_parse(value: &str) -> #{Result} { + match Self::from(value) { + ##[allow(deprecated)] + Self::Unknown(_) => #{Err}(#{UnknownVariantError}::new(value)), + known => Ok(known), + } } } - } - """, - *preludeScope, - "UnknownVariantError" to unknownVariantError(), - ) + """, + *preludeScope, + "UnknownVariantError" to unknownVariantError(), + ) + } } override fun additionalDocs(context: EnumGeneratorContext): Writable = writable { diff --git a/rust-runtime/aws-smithy-runtime-api/external-types.toml b/rust-runtime/aws-smithy-runtime-api/external-types.toml index 904961adb2..8d49ecdb57 100644 --- a/rust-runtime/aws-smithy-runtime-api/external-types.toml +++ b/rust-runtime/aws-smithy-runtime-api/external-types.toml @@ -8,5 +8,6 @@ allowed_external_types = [ "http::header::value::HeaderValue", "http::request::Request", "http::response::Response", + "http::status::StatusCode", "http::uri::Uri", ] From a2da78e82cab1ee01219191c9463c3b05aef1fd1 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 15 Nov 2023 09:33:26 -0800 Subject: [PATCH 13/13] Tweak deprecation message --- .../error/OperationErrorGenerator.kt | 2 +- .../generators/error/ServiceErrorGenerator.kt | 25 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt index 9cb3082c20..280b06c2ca 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/OperationErrorGenerator.kt @@ -88,7 +88,7 @@ class OperationErrorGenerator( #{deprecation} Unhandled(#{Unhandled}), """, - "deprecation" to writable { renderUnhandledErrorDeprecation() }, + "deprecation" to writable { renderUnhandledErrorDeprecation(runtimeConfig, errorSymbol.name) }, "Unhandled" to unhandledError(runtimeConfig), ) } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt index c4bb7ae114..5a33941845 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGenerator.kt @@ -232,7 +232,7 @@ class ServiceErrorGenerator( rust("${sym.name}(#T),", sym) } docs("An unexpected error occurred (e.g., invalid JSON returned by the service or an unknown error code).") - renderUnhandledErrorDeprecation() + renderUnhandledErrorDeprecation(codegenContext.runtimeConfig, "Error") rust("Unhandled(#T)", unhandledError(codegenContext.runtimeConfig)) } } @@ -261,19 +261,28 @@ fun unhandledError(rc: RuntimeConfig): RuntimeType = RuntimeType.forInlineFun( } """, "BoxError" to RuntimeType.smithyRuntimeApi(rc).resolve("box_error::BoxError"), - "deprecation" to writable { renderUnhandledErrorDeprecation() }, + "deprecation" to writable { renderUnhandledErrorDeprecation(rc) }, "ErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ErrorMetadata"), "ProvideErrorMetadata" to RuntimeType.smithyTypes(rc).resolve("error::metadata::ProvideErrorMetadata"), ) } -fun RustWriter.renderUnhandledErrorDeprecation() { +fun RustWriter.renderUnhandledErrorDeprecation(rc: RuntimeConfig, errorName: String? = null) { + val link = if (errorName != null) { + "##impl-ProvideErrorMetadata-for-$errorName" + } else { + "#{ProvideErrorMetadata}" + } val message = """ - Don't directly match on `Unhandled`. Instead, match using the catch all matcher (e.g., `_`), - give it a name like `err`, and use the `ProvideErrorMetadata` trait to retrieve the error - code and message. For example: `err => println!("code: {}, message: {}", err.code(), err.message())` + Matching `Unhandled` directly is not forwards compatible. Instead, match using a + variable wildcard pattern and check `.code()`:
+    `err if err.code() == Some("SpecificExceptionCode") => { /* handle the error */ }`
+ See [`ProvideErrorMetadata`]($link) for what information is available for the error. """.trimIndent() // `.dq()` doesn't quite do what we want here since we actually want a Rust multi-line string - val messageEscaped = message.replace("\"", "\\\"").replace("\n", " \\\n") - rust("""##[deprecated(note = "$messageEscaped")]""") + val messageEscaped = message.replace("\"", "\\\"").replace("\n", " \\\n").replace("
", "\n") + rustTemplate( + """##[deprecated(note = "$messageEscaped")]""", + "ProvideErrorMetadata" to RuntimeType.provideErrorMetadataTrait(rc), + ) }