refactor: delegate exception error messages to exception base class logic (#1044)

Author: aajtodd

.changes/f274caeb-9450-45b1-935c-5f61905de53b.json

@@ -0,0 +1,5 @@
+{
+    "id": "f274caeb-9450-45b1-935c-5f61905de53b",
+    "type": "misc",
+    "description": "Refactor exception codegen to delegate message field to exception base class"
+}

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt

@@ -66,33 +66,29 @@ class StructureGenerator(
         // generate the immutable properties that are set from a builder
         sortedMembers.forEach {
             val (memberName, memberSymbol) = memberNameSymbolIndex[it]!!
+            // Throwable.message is handled special and passed as a constructor parameter to the parent exception base class
+            if (shape.isError && memberName == "message") {
+                val targetShape = model.expectShape(it.target)
+                if (!targetShape.isStringShape) {
+                    throw CodegenException("message is a reserved name for exception types and cannot be used for any other property")
+                }
+                return@forEach
+            }
             writer.renderMemberDocumentation(model, it)
             writer.renderAnnotations(it)
-            renderImmutableProperty(it, memberName, memberSymbol)
+            renderImmutableProperty(memberName, memberSymbol)
         }
     }
 
-    private fun renderImmutableProperty(memberShape: MemberShape, memberName: String, memberSymbol: Symbol) {
-        // override Throwable's message property
-        val prefix = if (shape.isError && memberName == "message") {
-            val targetShape = model.expectShape(memberShape.target)
-            if (!targetShape.isStringShape) {
-                throw CodegenException("message is a reserved name for exception types and cannot be used for any other property")
-            }
-            "override"
-        } else {
-            "public"
-        }
-
+    private fun renderImmutableProperty(memberName: String, memberSymbol: Symbol) {
         if (memberSymbol.isRequiredWithNoDefault) {
             writer.write(
-                """#1L val #2L: #3F = requireNotNull(builder.#2L) { "A non-null value must be provided for #2L" }""",
-                prefix,
+                """public val #1L: #2F = requireNotNull(builder.#1L) { "A non-null value must be provided for #1L" }""",
                 memberName,
                 memberSymbol,
             )
         } else {
-            writer.write("#1L val #2L: #3F = builder.#2L", prefix, memberName, memberSymbol)
+            writer.write("public val #1L: #2F = builder.#1L", memberName, memberSymbol)
         }
     }
 
@@ -316,10 +312,16 @@ class StructureGenerator(
         val exceptionBaseClass = ExceptionBaseClassGenerator.baseExceptionSymbol(ctx.settings)
         writer.addImport(exceptionBaseClass)
 
+        val superParam = shape.members().find {
+            symbolProvider.toMemberName(it) == "message"
+        }?.let { "builder.message" } ?: ""
+
         writer.openBlock(
-            "#L class #T private constructor(builder: Builder) : ${exceptionBaseClass.name}() {",
+            "#L class #T private constructor(builder: Builder) : #L(#L) {",
             ctx.settings.api.visibility,
             symbol,
+            exceptionBaseClass.name,
+            superParam,
         )
             .write("")
             .call { renderImmutableProperties() }

codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ExceptionGeneratorTest.kt

@@ -86,12 +86,53 @@ class ExceptionGeneratorTest {
         clientErrorTestContents.shouldContainWithDiff(expectedClientClassDecl)
 
         val expectedServerClassDecl = """
-            class InternalServerException private constructor(builder: Builder) : TestException() {
+            class InternalServerException private constructor(builder: Builder) : TestException(builder.message) {
         """.trimIndent()
 
         serverErrorTestContents.shouldContainWithDiff(expectedServerClassDecl)
     }
 
+    @Test
+    fun `throwable message special cases`() {
+        val model = """
+        @httpError(500)
+        @error("server")
+        structure CapitalizedMessageMemberException {
+            Message: String
+        }
+        
+        @httpError(500)
+        @error("server")
+        structure NoMessageMemberException { }
+        """
+            .prependNamespaceAndService(version = "2.0")
+            .toSmithyModel()
+
+        val expectedCapMessageClassDecl = """
+            class CapitalizedMessageMemberException private constructor(builder: Builder) : TestException(builder.message) {
+        """.trimIndent()
+
+        val expectedNoMessageClassDecl = """
+            class NoMessageMemberException private constructor(builder: Builder) : TestException() {
+        """.trimIndent()
+
+        val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
+
+        listOf(
+            "com.test#CapitalizedMessageMemberException" to expectedCapMessageClassDecl,
+            "com.test#NoMessageMemberException" to expectedNoMessageClassDecl,
+        ).forEach { (shapeId, expected) ->
+
+            val writer = KotlinWriter(TestModelDefault.NAMESPACE)
+            val struct = model.expectShape<StructureShape>(shapeId)
+            val renderingCtx = RenderingContext(writer, struct, model, provider, model.defaultSettings())
+            StructureGenerator(renderingCtx).render()
+
+            val generated = writer.toString()
+            generated.shouldContainOnlyOnceWithDiff(expected)
+        }
+    }
+
     @Test
     fun `error generator sets error type correctly`() {
         val expectedClientClassDecl = "sdkErrorMetadata.attributes[ServiceErrorMetadata.ErrorType] = ErrorType.Client"
@@ -109,16 +150,6 @@ class ExceptionGeneratorTest {
         serverErrorTestContents.assertBalancedBracesAndParens()
     }
 
-    @Test
-    fun `error generator renders override with message member`() {
-        val expected = """
-    override val message: kotlin.String = requireNotNull(builder.message) { "A non-null value must be provided for message" }
-"""
-
-        serverErrorTestContents.shouldContainWithDiff(expected)
-        clientErrorTestContents.shouldNotContain(expected)
-    }
-
     @Test
     fun `error generator renders isRetryable`() {
         val expected = "sdkErrorMetadata.attributes[ErrorMetadata.Retryable] = true"

runtime/runtime-core/api/runtime-core.api

@@ -58,6 +58,7 @@ public class aws/smithy/kotlin/runtime/ServiceException : aws/smithy/kotlin/runt
 	public fun <init> (Ljava/lang/String;)V
 	public fun <init> (Ljava/lang/String;Ljava/lang/Throwable;)V
 	public fun <init> (Ljava/lang/Throwable;)V
+	public fun getMessage ()Ljava/lang/String;
 	public synthetic fun getSdkErrorMetadata ()Laws/smithy/kotlin/runtime/ErrorMetadata;
 	public fun getSdkErrorMetadata ()Laws/smithy/kotlin/runtime/ServiceErrorMetadata;
 }

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/Exceptions.kt

@@ -149,5 +149,8 @@ public open class ServiceException : SdkBaseException {
 
     public constructor(cause: Throwable?) : super(cause)
 
+    override val message: String?
+        get() = super.message ?: sdkErrorMetadata.errorMessage
+
     override val sdkErrorMetadata: ServiceErrorMetadata = ServiceErrorMetadata()
 }