remove println statements; add clarifying comment about CancellationE…

…xception
smithy-lang · Apr 17, 2024 · a7b39c7 · a7b39c7
1 parent 0b5d381
commit a7b39c7
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 6 deletions.
diff --git a/...client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt b/...client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt
@@ -55,8 +55,11 @@ public class OkHttpEngine(
         val responseTime = Instant.fromEpochMilliseconds(engineResponse.receivedResponseAtMillis)
 
         return OkHttpCall(request, response, requestTime, responseTime, callContext, engineCall).also { call ->
-            callContext.job.invokeOnCompletion { error ->
-                if (error != null) call.cancelInFlight()
+            callContext.job.invokeOnCompletion { cause ->
+                // If cause is non-null that means the job was cancelled (CancellationException) or failed (anything
+                // else). In both cases we need to ensure that the engine-side resources are cleaned up completely
+                // since they wouldn't otherwise be. https://github.com/smithy-lang/smithy-kotlin/issues/1061
+                if (cause != null) call.cancelInFlight()
                 engineResponse.body.close()
             }
         }

diff --git a/...ent-engines/test-suite/common/test/aws/smithy/kotlin/runtime/http/test/AsyncStressTest.kt b/...ent-engines/test-suite/common/test/aws/smithy/kotlin/runtime/http/test/AsyncStressTest.kt
@@ -102,17 +102,17 @@ class AsyncStressTest : AbstractEngineTest() {
             assertFailsWith<CancellationException> {
                 coroutineScope {
                     val parentScope = this
-
-                    println("Invoking call on ctx $coroutineContext")
                     val call = client.call(req)
 
                     val bytes = async {
                         delay(100.milliseconds)
-                        println("Body of type ${call.response.body} on ctx $coroutineContext")
+
                         try {
+                            // The server side is feeding us bytes very slowly. This shouldn't complete before the
+                            // parentScope cancellation happens below.
                             call.response.body.readAll()
                         } catch (e: Throwable) {
-                            // IllegalStateException: "Unbalanced enter/exit" will be thrown if body closed improperly
+                            // "IllegalStateException: Unbalanced enter/exit" will be thrown if body closed improperly
                             assertIsNot<IllegalStateException>(e)
                             null
                         }