refactor: make HttpCall context explicit (#939)

Author: aajtodd

.changes/5cd4e551-c1b1-486a-b2f2-e6d0066de913.json

@@ -0,0 +1,5 @@
+{
+    "id": "5cd4e551-c1b1-486a-b2f2-e6d0066de913",
+    "type": "misc",
+    "description": "**BREAKING**: Refactor HttpCall and HttpResponse to not be data classes and make the call context more explicit"
+}

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt

@@ -17,6 +17,7 @@ import software.amazon.smithy.kotlin.codegen.model.toSymbol
 object RuntimeTypes {
     object Http : RuntimeTypePackage(KotlinDependency.HTTP) {
         val HttpBody = symbol("HttpBody")
+        val HttpCall = symbol("HttpCall")
         val HttpMethod = symbol("HttpMethod")
         val ByteArrayContent = symbol("ByteArrayContent", subpackage = "content")
         val readAll = symbol("readAll")
@@ -41,7 +42,6 @@ object RuntimeTypes {
         }
 
         object Response : RuntimeTypePackage(KotlinDependency.HTTP, "response") {
-            val HttpCall = symbol("HttpCall")
             val HttpResponse = symbol("HttpResponse")
         }
     }

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

@@ -73,7 +73,7 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
      * The function should have the following signature:
      *
      * ```
-     * suspend fun throwFooOperationError(context: ExecutionContext, response: HttpResponse): Nothing {
+     * suspend fun throwFooOperationError(context: ExecutionContext, call: HttpCall): Nothing {
      *     <-- CURRENT WRITER CONTEXT -->
      * }
      * ```
@@ -494,19 +494,11 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
 
             reference(outputSymbol, SymbolReference.ContextOption.DECLARE)
         }
-        val operationDeserializerSymbols = setOf(
-            RuntimeTypes.HttpClient.Operation.HttpDeserialize,
-            RuntimeTypes.Http.Response.HttpResponse,
-        )
 
         val resolver = getProtocolHttpBindingResolver(ctx.model, ctx.service)
         val responseBindings = resolver.responseBindings(op)
         ctx.delegator.useSymbolWriter(deserializerSymbol) { writer ->
-            // import all of http, http.response , and serde packages. All serializers requires one or more of the symbols
-            // and most require quite a few. Rather than try and figure out which specific ones are used just take them
-            // all to ensure all the various DSL builders are available, etc
             writer
-                .addImport(operationDeserializerSymbols)
                 .write("")
                 .openBlock(
                     "internal class #T: #T<#T> {",
@@ -529,7 +521,7 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
         writer.addImport(RuntimeTypes.Http.isSuccess)
         writer.withBlock("if (!response.status.#T()) {", "}", RuntimeTypes.Http.isSuccess) {
             val errorHandlerFn = operationErrorHandler(ctx, op)
-            write("#T(context, response)", errorHandlerFn)
+            write("#T(context, call)", errorHandlerFn)
         }
     }
 
@@ -583,11 +575,12 @@ abstract class HttpBindingProtocolGenerator : ProtocolGenerator {
         writer
             .addImport(RuntimeTypes.Core.ExecutionContext)
             .openBlock(
-                "override suspend fun deserialize(context: #T, response: #T): #T {",
+                "override suspend fun deserialize(context: #T, call: #T): #T {",
                 RuntimeTypes.Core.ExecutionContext,
-                RuntimeTypes.Http.Response.HttpResponse,
+                RuntimeTypes.Http.HttpCall,
                 outputSymbol,
             )
+            .write("val response = call.response")
             .call {
                 if (outputSymbol.shape?.isError == false && op != null) {
                     // handle operation errors

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

@@ -365,9 +365,10 @@ internal class ConstantQueryStringOperationSerializer: HttpSerialize<ConstantQue
         val expectedContents = """
 internal class SmokeTestOperationDeserializer: HttpDeserialize<SmokeTestResponse> {
 
-    override suspend fun deserialize(context: ExecutionContext, response: HttpResponse): SmokeTestResponse {
+    override suspend fun deserialize(context: ExecutionContext, call: HttpCall): SmokeTestResponse {
+        val response = call.response
         if (!response.status.isSuccess()) {
-            throwSmokeTestError(context, response)
+            throwSmokeTestError(context, call)
         }
         val builder = SmokeTestResponse.Builder()
 

docs/design/resources/http-call-context.png

@@ -0,0 +1,63 @@
+# Structured Concurrency
+
+This document gives an overview of the [CoroutineScope](https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-coroutine-scope/)(s) 
+used by the SDK. It is meant to be a living document and provide additional context and guidance for SDK authors.
+
+## Definitions
+
+* **Coroutine** - An instance of a suspendable computation. A coroutine itself is represented by a Job. It is responsible for coroutine’s lifecycle, cancellation, and parent-child relations.
+* **CoroutineContext** - Persistent (immutable) context for a coroutine
+* **CoroutineScope** - Defines a scope for new coroutines which delimits the lifetime of the coroutine
+* **Structured Concurrency** - A principle expected by coroutines (in Kotlin) whereby new coroutines can only be launched into a specific CoroutineScope. This constraint ensures that coroutines are not leaked and that errors are propagated correctly. An outer scope cannot complete until all its child coroutines complete. 
+* **HttpCall** - A single HTTP request/response pair. 
+* **HttpClientEngine** - A component responsible for executing an HTTP request and returning an HttpCall
+* **ExecutionContext** - Operation scoped (mutable) context used to drive an operation to completion
+
+
+## Scopes
+
+The SDK has three implementations of `CoroutineScope`:
+
+1. [ExecutionContext](https://github.com/awslabs/smithy-kotlin/blob/main/runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/operation/ExecutionContext.kt)
+2. [HttpClientEngine](https://github.com/awslabs/smithy-kotlin/blob/main/runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/HttpClientEngine.kt)
+3. [HttpCall](https://github.com/awslabs/smithy-kotlin/blob/main/runtime/protocol/http/common/src/aws/smithy/kotlin/runtime/http/response/HttpCall.kt)
+    
+
+`ExecutionContext` implements `CoroutineScope` to provide a place for any background work to be done as part of implementing an operation. 
+Its scope begins and ends with an operation. The only place it is currently utilized is to launch background task(s) to process input event streams 
+(pull data from the customer input event stream, transform it, and write it to the HTTP body). 
+
+`HttpClientEngine` implements `CoroutineScope` and is used to launch HTTP requests. The scope of the engine is from creation to when it is closed.
+Making HTTP request tasks children of this scope means that an engine won’t be shutdown until in-flight requests are complete.
+The engine scope is a [SupervisorJob](https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-supervisor-job.html) such that individual HTTP requests can fail independent of one another without causing all requests to be cancelled.
+
+`HttpCall` implements `CoroutineScope` to provide a place for any background work required to fulfill an HTTP request/response. It is described
+in more detail in the following section.
+
+### HttpCall Scope
+
+Individual HTTP calls get their own `CoroutineContext` whose scope begins with executing the request and ends when `HttpCall::complete()` is invoked.
+This context/scope is referred to as the "call context". The call context is tied to the `HttpEngine` and is used primarily to launch background work
+required to fulfill executing the request and response. It is the parent to the `HttpClientEngine::roundTrip` invocation as well as any coroutines
+launched required to send the request body or fulfill the response body (e.g. reading off the wire and adapting to the the SDK I/O primitives). 
+
+Visually the parent/child relationships look like this:
+
+![HttpCall Context](resources/http-call-context.png)
+
+
+The call context is _not_ a child of the operation context it is invoked from. It is however configured (manually) to be cancelled if the caller cancels the operation (dotted red line).
+
+### FAQ
+
+**Why is the call context not a child of the operation context?**
+
+By the time `roundTrip` returns we will have executed the request and received everything but the response body. The only work that should be launched in the background as a child of the call context at this point is work required to fulfill the HTTP response body. Any errors in coroutines tied to the call context are surfaced through the HTTP body itself when it is read.
+
+Coroutines launched into the `HttpCall` scope are all meant to service executing a single HTTP request/response pair. It is logically consuming resources associated with the engine and as such is a child of the engine executing the request. It being a child of the engine also gives accurate accounting
+of in-flight requests and allows for a clean engine shutdown that doesn't abrubtly close resources in-use. 
+
+**Why is the call context the parent to the `roundTrip` invocation?**
+
+The `roundTrip` function is designed to return after the HTTP response headers are available. The body is not processed until deserialization or in the case of streaming responses when the caller consumes the body. As such the `roundTrip` function is responsible for only a subset of the overall HTTP
+call (request/response) lifecycle.

docs/design/structured-concurrency.md

@@ -5,7 +5,7 @@ kotlin.mpp.stability.nowarn=true
 kotlin.native.ignoreDisabledTargets=true
 
 # SDK
-sdkVersion=0.26.1-SNAPSHOT
+sdkVersion=0.27.0-SNAPSHOT
 
 # kotlin
-kotlinVersion=1.8.22
+kotlinVersion=1.8.22

gradle.properties

@@ -44,10 +44,6 @@ public suspend fun Flow<SdkBuffer>.asEventStreamHttpBody(scope: CoroutineScope):
         private var job: Job? = null
 
         override fun readFrom(): SdkByteReadChannel {
-            // FIXME - delaying launch here until the channel is consumed from the HTTP engine is a hacky way
-            //  of enforcing ordering to ensure the ExecutionContext is updated with the
-            //  AwsSigningAttributes.RequestSignature by the time the messages are collected and sign() is called
-
             // Although rare, nothing stops downstream consumers from invoking readFrom() more than once.
             // Only launch background collection task on first call
             if (job == null) {

runtime/protocol/aws-event-stream/common/src/aws/smithy/kotlin/runtime/awsprotocol/eventstream/FrameEncoder.kt

@@ -5,12 +5,12 @@
 
 package aws.smithy.kotlin.runtime.http.engine.crt
 
+import aws.smithy.kotlin.runtime.http.HttpCall
 import aws.smithy.kotlin.runtime.http.config.EngineFactory
 import aws.smithy.kotlin.runtime.http.engine.HttpClientEngine
 import aws.smithy.kotlin.runtime.http.engine.HttpClientEngineBase
 import aws.smithy.kotlin.runtime.http.engine.callContext
 import aws.smithy.kotlin.runtime.http.request.HttpRequest
-import aws.smithy.kotlin.runtime.http.response.HttpCall
 import aws.smithy.kotlin.runtime.io.internal.SdkDispatchers
 import aws.smithy.kotlin.runtime.operation.ExecutionContext
 import aws.smithy.kotlin.runtime.telemetry.logging.logger

runtime/protocol/http-client-engines/http-client-engine-crt/common/src/aws/smithy/kotlin/runtime/http/engine/crt/CrtHttpEngine.kt

@@ -7,9 +7,9 @@ package aws.smithy.kotlin.runtime.http.engine.crt
 
 import aws.smithy.kotlin.runtime.http.HttpMethod
 import aws.smithy.kotlin.runtime.http.SdkHttpClient
+import aws.smithy.kotlin.runtime.http.complete
 import aws.smithy.kotlin.runtime.http.request.HttpRequestBuilder
 import aws.smithy.kotlin.runtime.http.request.url
-import aws.smithy.kotlin.runtime.http.response.complete
 import aws.smithy.kotlin.runtime.httptest.TestWithLocalServer
 import aws.smithy.kotlin.runtime.net.Host
 import aws.smithy.kotlin.runtime.net.Scheme