diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index ab05df0124..ccd59ef313 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -431,3 +431,29 @@ message = "`Endpoint::immutable` now takes `impl AsRef` 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" diff --git a/codegen-core/common-test-models/rest-json-extras.smithy b/codegen-core/common-test-models/rest-json-extras.smithy index 65b7fcc8f1..b9946baa81 100644 --- a/codegen-core/common-test-models/rest-json-extras.smithy +++ b/codegen-core/common-test-models/rest-json-extras.smithy @@ -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", } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/RestJson.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/RestJson.kt index aa8e0f84b9..0a7234f548 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/RestJson.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/RestJson.kt @@ -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> = - 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 = diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt index 93ae58ce98..a865f27220 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt @@ -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, KFunction1> = - 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, + ) } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt index fa45a73ffd..a8a6e1608f 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt @@ -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,