From fc63800f6af21a9e6e44608e2ac705d8134da2e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antoine=20B=C3=BCsch?= Date: Mon, 17 Apr 2023 23:51:01 +1000 Subject: [PATCH] Implement `StdError::source()` for Error enum (#2564) ## Motivation and Context This is an attempt at fixing https://github.com/awslabs/aws-sdk-rust/issues/784. The service-level `Error` enum implements `std::error::Error` but does not implement its `source()` method. This means that an error library like `anyhow` or `eyre` won't be able to display the root cause of an error, which is especially problematic for the `Unhandled` variant. ## Description I modified `ServiceErrorGenerator` in the `codegen-client` crate and replaced the line that output `impl std::error::Error for Error {}` with an impl block that implements the `source()` method by delegating to the inner error structure. ## Testing I've added a simple unit test to `ServiceErrorGeneratorTest`. ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- CHANGELOG.next.toml | 12 +++ .../generators/error/ServiceErrorGenerator.kt | 11 ++- .../error/ServiceErrorGeneratorTest.kt | 74 ++++++++++++------- 3 files changed, 71 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index e07d004d2a..eb431fbb7a 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -11,6 +11,18 @@ # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} # author = "rcoh" +[[aws-sdk-rust]] +message = "Implement std::error::Error#source() properly for the service meta Error enum" +references = ["aws-sdk-rust#784"] +meta = { "breaking" = false, "tada" = false, "bug" = false } +author = "abusch" + +[[smithy-rs]] +message = "Implement std::error::Error#source() properly for the service meta Error enum" +references = ["aws-sdk-rust#784"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"} +author = "abusch" + [[aws-sdk-rust]] message = "The outputs for event stream operations (for example, S3's SelectObjectContent) now implement the `Sync` auto-trait." references = ["smithy-rs#2496"] 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 f461d59e05..c11fbfecf8 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 @@ -80,7 +80,16 @@ class ServiceErrorGenerator( errors.map { it.id }, ) } - rust("impl #T for Error {}", RuntimeType.StdError) + rustBlock("impl #T for Error", RuntimeType.StdError) { + rustBlock("fn source(&self) -> std::option::Option<&(dyn #T + 'static)>", RuntimeType.StdError) { + rustBlock("match self") { + allErrors.forEach { + rust("Error::${symbolProvider.toSymbol(it).name}(inner) => inner.source(),") + } + rust("Error::Unhandled(inner) => inner.source()") + } + } + } writeCustomizations(customizations, ErrorSection.ServiceErrorAdditionalTraitImpls(allErrors)) } crate.lib { rust("pub use error_meta::Error;") } diff --git a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGeneratorTest.kt b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGeneratorTest.kt index 1cbb274cfb..0f26fc7b42 100644 --- a/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGeneratorTest.kt +++ b/codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/error/ServiceErrorGeneratorTest.kt @@ -6,45 +6,48 @@ package software.amazon.smithy.rust.codegen.client.smithy.generators.error import org.junit.jupiter.api.Test +import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.core.testutil.integrationTest +import software.amazon.smithy.rust.codegen.core.testutil.unitTest +import software.amazon.smithy.rust.codegen.core.util.lookup internal class ServiceErrorGeneratorTest { - @Test - fun `top level errors are send + sync`() { - val model = """ - namespace com.example + private val model = """ + namespace com.example - use aws.protocols#restJson1 + use aws.protocols#restJson1 - @restJson1 - service HelloService { - operations: [SayHello], - version: "1" - } + @restJson1 + service HelloService { + operations: [SayHello], + version: "1" + } - @http(uri: "/", method: "POST") - operation SayHello { - input: EmptyStruct, - output: EmptyStruct, - errors: [SorryBusy, CanYouRepeatThat, MeDeprecated] - } + @http(uri: "/", method: "POST") + operation SayHello { + input: EmptyStruct, + output: EmptyStruct, + errors: [SorryBusy, CanYouRepeatThat, MeDeprecated] + } - structure EmptyStruct { } + structure EmptyStruct { } - @error("server") - structure SorryBusy { } + @error("server") + structure SorryBusy { } - @error("client") - structure CanYouRepeatThat { } + @error("client") + structure CanYouRepeatThat { } - @error("client") - @deprecated - structure MeDeprecated { } - """.asSmithyModel() + @error("client") + @deprecated + structure MeDeprecated { } + """.asSmithyModel() + @Test + fun `top level errors are send + sync`() { clientIntegrationTest(model) { codegenContext, rustCrate -> rustCrate.integrationTest("validate_errors") { rust( @@ -60,4 +63,25 @@ internal class ServiceErrorGeneratorTest { } } } + + @Test + fun `generates combined error enums`() { + clientIntegrationTest(model) { _, rustCrate -> + rustCrate.moduleFor(model.lookup("com.example#CanYouRepeatThat")) { + unitTest( + name = "generates_combined_error_enums", + test = """ + use std::error::Error as StdError; + use crate::Error; + use crate::operation::say_hello::SayHelloError; + + // Unhandled variants properly delegate source. + let error = Error::from(SayHelloError::unhandled("some other error")); + let source = error.source().expect("source should not be None"); + assert_eq!(format!("{}", source), "some other error"); + """, + ) + } + } + } }