fix: route JDK transport async dispatch failures through the future

JdkHttpTransport.executeAsync guarded only the request-adaptation call.
The dispatch kickoff that follows on the caller's thread —
HttpClient.sendAsync plus the bridgeAsyncResponse wiring — ran
unguarded. sendAsync does not guarantee that every failure reaches its
returned future: on a closed java.net.http.HttpClient (JDK 21+, where
the client is AutoCloseable) it throws synchronously, so the exception
escaped on the caller's thread and bypassed the future. That breaks the
method's documented contract that all errors arrive through the returned
CompletableFuture, and a future-composing caller's .exceptionally/.handle
would never observe such a throw.

Widen the try/catch to enclose the whole post-adaptation dispatch path so
any synchronous throw becomes a failed future. The OkHttp transport is
already correct here — newCall does no throwing work and dispatch
failures arrive via Callback.onFailure — so it is left unchanged apart
from a comment recording why its post-adapt path is future-safe.

The catch stays at Exception (not RuntimeException) in both transports:
the intent is that any non-fatal failure, including an unexpected adapter
bug such as an NPE, surfaces via the future; only Error / JVM-fatal
conditions propagate. The inline comments now state that breadth is
deliberate.

Both transports' async adaptation-failure tests now assert
future.isCompletedExceptionally() is already true on return (completion
is synchronous, not merely eventual) and assert a substring of the
adapter's message so an unrelated IllegalArgumentException cannot satisfy
the test. A new JDK test closes an AutoCloseable client and asserts the
synchronous sendAsync throw is delivered through the future.

Closes #120
Closes #121
Closes #122

Author: OmarAlJarrah

sdk-transport-jdkhttp/src/main/kotlin/org/dexpace/sdk/transport/jdkhttp/JdkHttpTransport.kt

@@ -138,21 +138,26 @@ public class JdkHttpTransport private constructor(
      * completions to release the body's connection back to the pool.
      */
     override fun executeAsync(request: Request): CompletableFuture<Response> {
-        val jdkRequest =
-            try {
-                requestAdapter.adapt(request, responseTimeout)
-            } catch (e: Exception) {
-                // The async contract is that errors arrive through the returned future. Request
-                // adaptation runs on the caller's thread and can throw (e.g. a CONNECT request the
-                // JDK client rejects), so route the failure into a failed future instead of
-                // throwing synchronously where a future-composing caller's .exceptionally/.handle
-                // would never observe it. Errors (OOM and other JVM-fatal conditions) are left to
-                // propagate up the caller's stack rather than be packaged into a future that may
-                // never be awaited.
-                return CompletableFuture.failedFuture<Response>(e)
-            }
-        val inFlight = client.sendAsync(jdkRequest, HttpResponse.BodyHandlers.ofInputStream())
-        return bridgeAsyncResponse(inFlight) { jdkResponse -> responseAdapter.adapt(request, jdkResponse) }
+        return try {
+            val jdkRequest = requestAdapter.adapt(request, responseTimeout)
+            // `sendAsync` is inside the guard too: it does not promise that every failure arrives
+            // through its returned future. On a closed `java.net.http.HttpClient` (JDK 21+, where
+            // the client is `AutoCloseable`) it throws synchronously on the caller's thread, which
+            // would bypass the future and break the error-delivery contract documented above.
+            val inFlight = client.sendAsync(jdkRequest, HttpResponse.BodyHandlers.ofInputStream())
+            bridgeAsyncResponse(inFlight) { jdkResponse -> responseAdapter.adapt(request, jdkResponse) }
+        } catch (e: Exception) {
+            // The async contract is that errors arrive through the returned future. The dispatch
+            // path above runs on the caller's thread and can throw — request adaptation rejecting a
+            // CONNECT request, `sendAsync` on a closed client, or an unexpected adapter bug such as
+            // an NPE — so route any of these into a failed future instead of throwing synchronously
+            // where a future-composing caller's .exceptionally/.handle would never observe it. The
+            // breadth is intentional: catching `Exception` (not `RuntimeException`) keeps a future
+            // adapter step's checked exception on the future too. Only `Error` (OOM and other
+            // JVM-fatal conditions) is left to propagate up the caller's stack rather than be
+            // packaged into a future that may never be awaited.
+            CompletableFuture.failedFuture<Response>(e)
+        }
     }
 
     /**

sdk-transport-jdkhttp/src/test/kotlin/org/dexpace/sdk/transport/jdkhttp/JdkHttpTransportTest.kt

@@ -162,11 +162,49 @@ class JdkHttpTransportTest {
                 .build()
         // Must return a future rather than throwing on the caller's thread.
         val future = transport.executeAsync(request)
+        // Completion is synchronous, not merely eventual: the future is already completed
+        // exceptionally on return, before anything is awaited.
+        assertTrue(
+            future.isCompletedExceptionally,
+            "adaptation failure must complete the future exceptionally synchronously on return",
+        )
         val ex = assertFailsWith<ExecutionException> { future.get(5, TimeUnit.SECONDS) }
         assertTrue(
             ex.cause is IllegalArgumentException,
             "adaptation failure must surface as the future's cause, was: ${ex.cause?.let { it::class }}",
         )
+        // Assert the message so an unrelated IllegalArgumentException cannot satisfy the test.
+        // RequestAdapter rejects CONNECT with "java.net.http.HttpClient does not support
+        // user-issued CONNECT requests."
+        assertTrue(
+            ex.cause?.message?.contains("does not support user-issued CONNECT requests") == true,
+            "expected the JDK CONNECT-rejection message, was: ${ex.cause?.message}",
+        )
+    }
+
+    @Test
+    fun `executeAsyncDeliversDispatchFailureThroughFuture`() {
+        // `sendAsync` does not promise that every failure arrives through its returned future:
+        // on a closed `java.net.http.HttpClient` it throws synchronously on the caller's thread.
+        // The dispatch path runs after a successful adaptation, so this exercises the post-adapt
+        // guard specifically — the failure must still arrive through the returned future, not be
+        // thrown on the caller's thread. `HttpClient` became `AutoCloseable` in JDK 21 (JEP 461);
+        // on older runtimes there is no close hook to trigger this, so the case is skipped.
+        val client = HttpClient.newHttpClient()
+        val closeable = client as? AutoCloseable ?: return // JDK 21+ only; no close hook earlier.
+        closeable.close()
+        val closedTransport = JdkHttpTransport.create(client)
+
+        val request = simpleGet("/async-dispatch-fail")
+        // Must return a future rather than throwing on the caller's thread.
+        val future = closedTransport.executeAsync(request)
+        assertTrue(
+            future.isCompletedExceptionally,
+            "a synchronous sendAsync throw must complete the future exceptionally synchronously",
+        )
+        // The closed-client throw is an unchecked exception; surface it as the future's cause
+        // rather than letting it escape executeAsync on the caller's thread.
+        assertFailsWith<ExecutionException> { future.get(5, TimeUnit.SECONDS) }
     }
 
     // -------- headers round-trip --------

sdk-transport-okhttp/src/main/kotlin/org/dexpace/sdk/transport/okhttp/OkHttpTransport.kt

@@ -116,11 +116,16 @@ public class OkHttpTransport private constructor(
                 // adaptation runs on the caller's thread and can throw (e.g. a method/body
                 // mismatch OkHttp rejects), so route the failure into a completed-exceptionally
                 // future instead of throwing synchronously where a future-composing caller's
-                // .exceptionally/.handle would never observe it. Errors (OOM and other JVM-fatal
-                // conditions) are left to propagate up the caller's stack rather than be packaged
-                // into a future that may never be awaited.
+                // .exceptionally/.handle would never observe it. The breadth is intentional:
+                // catching `Exception` (not `RuntimeException`) also funnels an unexpected adapter
+                // bug such as an NPE through the future so the caller can observe it. Only `Error`
+                // (OOM and other JVM-fatal conditions) is left to propagate up the caller's stack
+                // rather than be packaged into a future that may never be awaited.
                 return failedFuture(e)
             }
+        // The post-adaptation dispatch needs no guard: `newCall(...)` does no throwing work, and a
+        // dispatch failure (including a `RejectedExecutionException` from a shut-down dispatcher)
+        // is delivered through `Callback.onFailure` below, so it already reaches the future.
         val call = client.newCall(okRequest)
         val future = CompletableFuture<Response>()
         call.enqueue(

sdk-transport-okhttp/src/test/kotlin/org/dexpace/sdk/transport/okhttp/OkHttpTransportTest.kt

@@ -207,11 +207,24 @@ class OkHttpTransportTest {
                 .build()
         // Must return a future rather than throwing on the caller's thread.
         val future = transport.executeAsync(request)
+        // Completion is synchronous, not merely eventual: the future is already completed
+        // exceptionally on return, before anything is awaited.
+        assertTrue(
+            future.isCompletedExceptionally,
+            "adaptation failure must complete the future exceptionally synchronously on return",
+        )
         val ex = assertFailsWith<ExecutionException> { future.get(5, TimeUnit.SECONDS) }
         assertTrue(
             ex.cause is IllegalArgumentException,
             "adaptation failure must surface as the future's cause, was: ${ex.cause?.let { it::class }}",
         )
+        // Assert the message so an unrelated IllegalArgumentException cannot satisfy the test.
+        // OkHttp's Request.Builder.method rejects a body on GET with "<method> must not have a
+        // request body."
+        assertTrue(
+            ex.cause?.message?.contains("must not have a request body") == true,
+            "expected OkHttp's body-on-GET rejection message, was: ${ex.cause?.message}",
+        )
     }
 
     // -------- headers round-trip --------