Unverified Commit c187e3d5 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Render full shape IDs in server error responses (#1982) 

RestJson1 implementations can denote errors in responses in several
ways. New server-side protocol implementations MUST use a header field
named `X-Amzn-Errortype`.

Note that the spec says that implementations SHOULD strip the error
shape ID's namespace. However, our server implementation renders the
full shape ID (including namespace), since some existing clients rely on
it to deserialize the error shape and fail if only the shape name is
present. This is compliant with the spec, see
https://github.com/awslabs/smithy/pull/1493.

See https://github.com/awslabs/smithy/issues/1494 too.
parent 1f405f63

CHANGELOG.next.toml

+26 −0
Original line number Diff line number Diff line
@@ -431,3 +431,29 @@ message = "`Endpoint::immutable` now takes `impl AsRef<str>` instead of `Uri`. F 
references = ["smithy-rs#1984", "smithy-rs#1496"]

meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }

author = "jdisanti"



[[smithy-rs]]

message = """

[RestJson1](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-restjson1-protocol.html#operation-error-serialization) server SDKs now serialize the [full shape ID](https://smithy.io/2.0/spec/model.html#shape-id) (including namespace) in operation error responses.



Example server error response before:



```

HTTP/1.1 400 Bad Request

content-type: application/json

x-amzn-errortype: InvalidRequestException

...

```



Example server error response now:



```

HTTP/1.1 400 Bad Request

content-type: application/json

x-amzn-errortype: com.example.service#InvalidRequestException

...

```

"""

references = ["smithy-rs#1982"]

meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "server" }

author = "david-perez"

codegen-core/common-test-models/rest-json-extras.smithy

+6 −2
Original line number Diff line number Diff line
@@ -81,12 +81,16 @@ service RestJsonExtras { 
        appliesTo: "client",

    },

    {

        documentation: "Upper case error modeled lower case",

        documentation: """

            Upper case error modeled lower case.

            Servers render the full shape ID (including namespace), since some

            existing clients rely on it to deserialize the error shape and fail

            if only the shape name is present.""",

        id: "ServiceLevelErrorServer",

        protocol: "aws.protocols#restJson1",

        code: 500,

        body: "{}",

        headers: { "X-Amzn-Errortype": "ExtraError" },

        headers: { "X-Amzn-Errortype": "aws.protocoltests.restjson#ExtraError" },

        params: {},

        appliesTo: "server",

    }

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/RestJson.kt

+7 −1
Original line number Diff line number Diff line
@@ -83,9 +83,15 @@ open class RestJson(val codegenContext: CodegenContext) : Protocol { 
    /**

     * RestJson1 implementations can denote errors in responses in several ways.

     * New server-side protocol implementations MUST use a header field named `X-Amzn-Errortype`.

     *

     * Note that the spec says that implementations SHOULD strip the error shape ID's namespace.

     * However, our server implementation renders the full shape ID (including namespace), since some

     * existing clients rely on it to deserialize the error shape and fail if only the shape name is present.

     * This is compliant with the spec, see https://github.com/awslabs/smithy/pull/1493.

     * See https://github.com/awslabs/smithy/issues/1494 too.

     */

    override fun additionalErrorResponseHeaders(errorShape: StructureShape): List<Pair<String, String>> =

        listOf("x-amzn-errortype" to errorShape.id.name)

        listOf("x-amzn-errortype" to errorShape.id.toString())



    override fun structuredDataParser(operationShape: OperationShape): StructuredDataParserGenerator {

        fun builderSymbol(shape: StructureShape): Symbol =

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt

+13 −1
Original line number Diff line number Diff line
@@ -1170,6 +1170,13 @@ class ServerProtocolTestGenerator( 
        ): HttpRequestTestCase =

            testCase.toBuilder().putHeader("x-amz-target", "JsonProtocol.${operationShape.id.name}").build()



        private fun fixRestJsonInvalidGreetingError(testCase: HttpResponseTestCase): HttpResponseTestCase =

            testCase.toBuilder().putHeader("X-Amzn-Errortype", "aws.protocoltests.restjson#InvalidGreeting").build()

        private fun fixRestJsonEmptyComplexErrorWithNoMessage(testCase: HttpResponseTestCase): HttpResponseTestCase =

            testCase.toBuilder().putHeader("X-Amzn-Errortype", "aws.protocoltests.restjson#ComplexError").build()

        private fun fixRestJsonComplexErrorWithNoMessage(testCase: HttpResponseTestCase): HttpResponseTestCase =

            testCase.toBuilder().putHeader("X-Amzn-Errortype", "aws.protocoltests.restjson#ComplexError").build()



        // These are tests whose definitions in the `awslabs/smithy` repository are wrong.

        // This is because they have not been written from a server perspective, and as such the expected `params` field is incomplete.

        // TODO(https://github.com/awslabs/smithy-rs/issues/1288): Contribute a PR to fix them upstream.
@@ -1246,6 +1253,11 @@ class ServerProtocolTestGenerator( 
        )



        private val BrokenResponseTests: Map<Pair<String, String>, KFunction1<HttpResponseTestCase, HttpResponseTestCase>> =

            mapOf()

            // TODO(https://github.com/awslabs/smithy/issues/1494)

            mapOf(

                Pair(RestJson, "RestJsonInvalidGreetingError") to ::fixRestJsonInvalidGreetingError,

                Pair(RestJson, "RestJsonEmptyComplexErrorWithNoMessage") to ::fixRestJsonEmptyComplexErrorWithNoMessage,

                Pair(RestJson, "RestJsonComplexErrorWithNoMessage") to ::fixRestJsonComplexErrorWithNoMessage,

            )

    }

}

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt

+3 −1
Original line number Diff line number Diff line
@@ -33,6 +33,8 @@ import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter 
import software.amazon.smithy.rust.codegen.core.rustlang.Writable

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

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

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

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

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

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

import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate
@@ -636,7 +638,7 @@ private class ServerHttpBoundProtocolTraitImplGenerator( 
                    builder = #{header_util}::set_response_header_if_absent(

                        builder,

                        http::header::HeaderName::from_static("$headerName"),

                        "$headerValue"

                        "${escape(headerValue)}"

                    );

                    """,

                    *codegenScope,