Unverified Commit ec0b06d1 authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

Top level error shape improvements (#172) 

* Fallback to a generic error representation for unmodeled errors

* DRY out some dependencies

* Fix name is instant_iso8601 resource

* Top level error shape improvements

This commit adds `code` and `message` as inherent methods to top level error shapes (eg. `ListTablesError`). If the error is unmodeled, it uses
downcasting to extract a message.

* Fix clippy warning
parent c9659093

codegen-test/dynamo-it/Cargo.lock

+31 −12
Original line number Diff line number Diff line
@@ -12,12 +12,12 @@ version = "0.0.1" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "85293e31cbf209322718ff220c48470f5dd31f39f652220bba6f3ceb2579cc81"

dependencies = [

 "bytes",

 "bytes 0.5.6",

 "chrono",

 "eliza_error",

 "hex",

 "http",

 "http-body",

 "http-body 0.3.1",

 "httparse",

 "hyper",

 "ring",
@@ -44,6 +44,12 @@ version = "0.5.6" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "0e4cec68f03f32e44924783795810fa50a7035d8c8ebe78580ad7e6c703fba38"



[[package]]

name = "bytes"

version = "1.0.1"

source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "b700ce4376041dcd0a327fd0097c41095743c4c8af8887265942faf1100bd040"



[[package]]

name = "cc"

version = "1.0.66"
@@ -188,7 +194,7 @@ version = "0.2.7" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "5e4728fd124914ad25e99e3d15a9361a879f6620f63cb56bbb08f95abb97a535"

dependencies = [

 "bytes",

 "bytes 0.5.6",

 "fnv",

 "futures-core",

 "futures-sink",
@@ -225,11 +231,11 @@ checksum = "644f9158b2f133fd50f5fb3242878846d9eb792e445c893805ff0e3824006e35" 


[[package]]

name = "http"

version = "0.2.1"

version = "0.2.3"

source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "28d569972648b2c512421b5f2a405ad6ac9666547189d0c5477a3f200f3e02f9"

checksum = "7245cd7449cc792608c3c8a9eaf69bd4eabbabf802713748fd739c98b82f0747"

dependencies = [

 "bytes",

 "bytes 1.0.1",

 "fnv",

 "itoa",

]
@@ -240,7 +246,17 @@ version = "0.3.1" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "13d5ff830006f7646652e057693569bfe0d51760c0085a071769d142a205111b"

dependencies = [

 "bytes",

 "bytes 0.5.6",

 "http",

]



[[package]]

name = "http-body"

version = "0.4.0"

source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "2861bd27ee074e5ee891e8b539837a9430012e249d7f0ca2d795650f579c1994"

dependencies = [

 "bytes 1.0.1",

 "http",

]
@@ -262,13 +278,13 @@ version = "0.13.9" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "f6ad767baac13b44d4529fcf58ba2cd0995e36e7b435bc5b039de6f47e880dbf"

dependencies = [

 "bytes",

 "bytes 0.5.6",

 "futures-channel",

 "futures-core",

 "futures-util",

 "h2",

 "http",

 "http-body",

 "http-body 0.3.1",

 "httparse",

 "httpdate",

 "itoa",
@@ -307,7 +323,7 @@ version = "0.1.0" 
dependencies = [

 "aws-sigv4",

 "http",

 "http-body",

 "http-body 0.3.1",

 "hyper",

 "pin-utils",

 "tokio",
@@ -708,6 +724,9 @@ checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" 
name = "smithy-http"

version = "0.0.1"

dependencies = [

 "bytes 1.0.1",

 "http",

 "http-body 0.4.0",

 "smithy-types",

]
@@ -779,7 +798,7 @@ version = "0.2.24" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "099837d3464c16a808060bb3f02263b412f6fafcb5d01c533d309985fbeebe48"

dependencies = [

 "bytes",

 "bytes 0.5.6",

 "fnv",

 "futures-core",

 "iovec",
@@ -814,7 +833,7 @@ version = "0.3.1" 
source = "registry+https://github.com/rust-lang/crates.io-index"

checksum = "be8242891f2b6cbef26a2d7e8605133c2c554cd35b3e4948ea892d6d68436499"

dependencies = [

 "bytes",

 "bytes 0.5.6",

 "futures-core",

 "futures-sink",

 "log",

codegen-test/dynamo-it/src/main.rs

+3 −2
Original line number Diff line number Diff line
@@ -58,7 +58,7 @@ async fn main() -> Result<(), Box<dyn Error>> { 
            .build()])

        .provisioned_throughput(

            ProvisionedThroughput::builder()

                .read_capacity_units(100)

                //.read_capacity_units(100)

                .write_capacity_units(100)

                .build(),

        )
@@ -76,7 +76,8 @@ async fn main() -> Result<(), Box<dyn Error>> { 
            );

            println!("{} was created", table_name);

        }

        _ => println!("{:?}", response.raw),

        Some(Err(e)) => println!("{}", e),

        _ => println!("something else")

    }



    let tables = io_v0::dispatch!(

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RuntimeTypes.kt

+3 −0
Original line number Diff line number Diff line
@@ -66,6 +66,9 @@ data class RuntimeType(val name: String?, val dependency: RustDependency?, val n 
        fun Instant(runtimeConfig: RuntimeConfig) =

            RuntimeType("Instant", CargoDependency.SmithyTypes(runtimeConfig), "${runtimeConfig.cratePrefix}_types")



        fun GenericError(runtimeConfig: RuntimeConfig) =

            RuntimeType("Error", CargoDependency.SmithyTypes(runtimeConfig), "${runtimeConfig.cratePrefix}_types")



        fun Blob(runtimeConfig: RuntimeConfig) =

            RuntimeType("Blob", CargoDependency.SmithyTypes(runtimeConfig), "${runtimeConfig.cratePrefix}_types")

codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/CombinedErrorGenerator.kt

+93 −12
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ 


package software.amazon.smithy.rust.codegen.smithy.generators



import software.amazon.smithy.codegen.core.Symbol

import software.amazon.smithy.codegen.core.SymbolProvider

import software.amazon.smithy.model.Model

import software.amazon.smithy.model.knowledge.OperationIndex
@@ -13,9 +14,13 @@ import software.amazon.smithy.rust.codegen.rustlang.Attribute 
import software.amazon.smithy.rust.codegen.rustlang.Derives

import software.amazon.smithy.rust.codegen.rustlang.RustMetadata

import software.amazon.smithy.rust.codegen.rustlang.RustWriter

import software.amazon.smithy.rust.codegen.rustlang.Writable

import software.amazon.smithy.rust.codegen.rustlang.rust

import software.amazon.smithy.rust.codegen.rustlang.rustBlock

import software.amazon.smithy.rust.codegen.rustlang.writable

import software.amazon.smithy.rust.codegen.smithy.RuntimeType

import software.amazon.smithy.rust.codegen.smithy.RustSymbolProvider

import software.amazon.smithy.rust.codegen.smithy.generators.config.Section



/**

 * For a given Operation ([this]), return the symbol referring to the unified error? This can be used
@@ -38,7 +43,7 @@ fun OperationShape.errorSymbol(symbolProvider: SymbolProvider): RuntimeType { 
 */

class CombinedErrorGenerator(

    model: Model,

    private val symbolProvider: SymbolProvider,

    private val symbolProvider: RustSymbolProvider,

    private val operation: OperationShape

) {
@@ -67,12 +72,8 @@ class CombinedErrorGenerator( 
        }

        writer.rustBlock("impl #T for ${symbol.name}", RuntimeType.StdFmt("Display")) {

            rustBlock("fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result") {

                rustBlock("match self") {

                    errors.forEach {

                        val errorSymbol = symbolProvider.toSymbol(it)

                        rust("""${symbol.name}::${errorSymbol.name}(inner) => inner.fmt(f),""")

                    }

                    rust("${symbol.name}::Unhandled(inner) => inner.fmt(f)")

                delegateToVariants {

                    writable { rust("_inner.fmt(f)") }

                }

            }

        }
@@ -81,16 +82,96 @@ class CombinedErrorGenerator( 
            writer.rustBlock("pub fn unhandled<E: Into<Box<dyn #T>>>(err: E) -> Self", RuntimeType.StdError) {

                write("${symbol.name}::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.

            writer.rustBlock("pub fn message(&self) -> Option<&str>") {

                delegateToVariants {

                    when (it) {

                        is VariantMatch.Generic, is VariantMatch.Modeled -> writable { rust("_inner.message()") }

                        else -> writable { rust("None") }

                    }

                }

            }



            writer.rustBlock("pub fn code(&self) -> Option<&str>") {

                delegateToVariants {

                    when (it) {

                        is VariantMatch.Unhandled -> writable { rust("None") }

                        is VariantMatch.Modeled -> writable { rust("Some(_inner.code())") }

                        is VariantMatch.Generic -> writable { rust("_inner.code()") }

                    }

                }

            }

        }



        writer.rustBlock("impl #T for ${symbol.name}", RuntimeType.StdError) {

            rustBlock("fn source(&self) -> Option<&(dyn #T + 'static)>", RuntimeType.StdError) {

                delegateToVariants {

                    writable {

                        when (it) {

                            is VariantMatch.Unhandled -> rust("Some(_inner.as_ref())")

                            is VariantMatch.Generic, is VariantMatch.Modeled -> rust("Some(_inner)")

                        }

                    }

                }

            }

        }

    }



    sealed class VariantMatch(name: String) : Section(name) {

        object Unhandled : VariantMatch("Unhandled")

        object Generic : VariantMatch("Generic")

        data class Modeled(val symbol: Symbol) : VariantMatch("Modeled")

    }



    /**

     * Generates code to delegate behavior to the variants, for example:

     * ```rust

     *  match self {

     *    GreetingWithErrorsError::InvalidGreeting(_inner) => inner.fmt(f),

     *    GreetingWithErrorsError::ComplexError(_inner) => inner.fmt(f),

     *    GreetingWithErrorsError::FooError(_inner) => inner.fmt(f),

     *    GreetingWithErrorsError::Unhandled(_inner) => match inner.downcast_ref::<::smithy_types::Error>() {

     *      Some(_inner) => inner.message(),

     *      None => None,

     *    }

     *  }

     *  ```

     *

     * [handler] is passed an instance of [VariantMatch]—a [writable] should be returned containing the content to be

     * written for this variant.

     *

     *  The field will always be bound as `_inner`.

     */

    private fun RustWriter.delegateToVariants(

        handler: (VariantMatch) -> Writable

    ) {

        val errors = operationIndex.getErrors(operation)

        val symbol = operation.errorSymbol(symbolProvider)

        val genericError = RuntimeType.GenericError(symbolProvider.config().runtimeConfig)

        rustBlock("match self") {

            errors.forEach {

                val errorSymbol = symbolProvider.toSymbol(it)

                        rust("""${symbol.name}::${errorSymbol.name}(inner) => Some(inner),""")

                rust("""${symbol.name}::${errorSymbol.name}(_inner) => """)

                handler(VariantMatch.Modeled(errorSymbol))(this)

                write(",")

            }

            val genericHandler = handler(VariantMatch.Generic)

            val unhandledHandler = handler(VariantMatch.Unhandled)

            rustBlock("${symbol.name}::Unhandled(_inner) =>") {

                if (genericHandler != unhandledHandler) {

                    rustBlock("match _inner.downcast_ref::<#T>()", genericError) {

                        rustBlock("Some(_inner) => ") {

                            genericHandler(this)

                        }

                        rustBlock("None => ") {

                            unhandledHandler(this)

                        }

                    }

                    rust("${symbol.name}::Unhandled(inner) => Some(inner.as_ref())")

                } else {

                    // If the handlers are the same, skip the downcast

                    genericHandler(this)

                }

            }

        }

codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/CombinedErrorGeneratorTest.kt

+15 −0
Original line number Diff line number Diff line
@@ -49,6 +49,21 @@ internal class CombinedErrorGeneratorTest { 
            """

            let error = GreetingError::InvalidGreeting(InvalidGreeting::builder().message("an error").build());

            assert_eq!(format!("{}", error), "InvalidGreeting: an error");

            assert_eq!(error.message(), Some("an error"));

            assert_eq!(error.code(), Some("InvalidGreeting"));





            // unhandled variants properly delegate message

            let error = GreetingError::Unhandled(Box::new(smithy_types::Error {

               code: None,

               message: Some("hello".to_string()),

               request_id: None

            }));

            assert_eq!(error.message(), Some("hello"));



            let error = GreetingError::unhandled("some other error");

            assert_eq!(error.message(), None);

            assert_eq!(error.code(), None);

        """

        )

    }