test: deterministically exercise the JDK transport's async dispatch guard

The async dispatch-failure test closed a java.net.http.HttpClient to provoke a
synchronous sendAsync throw, but two things undercut it: the JDK client returns
an already-failed future for a closed client rather than throwing (so the bridge,
not executeAsync's guard, handled it), and on this module's JDK 11 test runtime
HttpClient is not AutoCloseable, so the test took its early-return skip path and
asserted nothing on every run.

Replace it with an injected HttpClient whose sendAsync throws on the caller's
thread. That drives the post-adaptation guard directly and deterministically on
every JDK; narrowing the guard back to wrap only request adaptation makes the
test fail, confirming it pins the behaviour.

Also correct the executeAsync comments, which stated as fact that sendAsync
throws synchronously on a closed client. The guard is defence-in-depth for a
client that throws on dispatch — not a description of the stock JDK client,
which delivers such failures through the returned future.

Author: OmarAlJarrah

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

@@ -140,22 +140,26 @@ public class JdkHttpTransport private constructor(
     override fun executeAsync(request: Request): CompletableFuture<Response> {
         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.
+            // `sendAsync` is inside the guard too. Its contract does not promise that every failure
+            // is delivered through the returned future: the JDK's own Javadoc permits a synchronous
+            // `IllegalArgumentException` for a request it rejects, and a custom or future
+            // `HttpClient` is free to throw on the caller's thread instead of returning a failed
+            // future. Guarding the dispatch keeps the error-delivery contract documented above
+            // intact whichever way a failure surfaces. (Today's stock JDK client packages such
+            // failures into an already-failed future — e.g. on a closed client — which the bridge
+            // propagates and so never reaches this catch; the guard is for the throwing case.)
             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.
+            // CONNECT request, a synchronous `sendAsync` rejection, 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

@@ -24,22 +24,31 @@ import org.dexpace.sdk.io.OkioIoProvider
 import org.dexpace.sdk.transport.jdkhttp.internal.BodyPublishers
 import java.io.ByteArrayOutputStream
 import java.io.IOException
+import java.net.Authenticator
+import java.net.CookieHandler
 import java.net.InetSocketAddress
+import java.net.ProxySelector
 import java.net.ServerSocket
 import java.net.URL
 import java.net.http.HttpClient
 import java.net.http.HttpRequest
+import java.net.http.HttpResponse
 import java.nio.ByteBuffer
 import java.security.MessageDigest
 import java.time.Duration
+import java.util.Optional
 import java.util.concurrent.CancellationException
+import java.util.concurrent.CompletableFuture
 import java.util.concurrent.CompletionException
 import java.util.concurrent.CountDownLatch
 import java.util.concurrent.ExecutionException
+import java.util.concurrent.Executor
 import java.util.concurrent.Flow
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicReference
+import javax.net.ssl.SSLContext
+import javax.net.ssl.SSLParameters
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertContentEquals
@@ -183,28 +192,32 @@ class JdkHttpTransportTest {
     }
 
     @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")
+    fun `executeAsyncRoutesSynchronousDispatchThrowThroughFuture`() {
+        // `sendAsync`'s contract does not promise that every failure is delivered through the
+        // returned future — a client may throw synchronously on the caller's thread. The dispatch
+        // path runs after a successful adaptation, so this injects a client whose `sendAsync`
+        // throws to exercise the post-adapt guard deterministically on every JDK. (The stock JDK
+        // client instead returns an already-failed future for, e.g., a closed client; the bridge
+        // propagates that and it never reaches the guard, so a real closed client cannot prove
+        // this path. The test double does.) The failure must arrive through the returned future,
+        // never escape executeAsync on the caller's thread.
+        val boom = IllegalStateException("synchronous dispatch failure")
+        val throwingTransport = JdkHttpTransport.create(SyncThrowingDispatchClient(boom))
+
         // Must return a future rather than throwing on the caller's thread.
-        val future = closedTransport.executeAsync(request)
+        val future = throwingTransport.executeAsync(simpleGet("/async-dispatch-throw"))
+        // Completion is synchronous, not merely eventual: the future is already completed
+        // exceptionally on return, before anything is awaited.
         assertTrue(
             future.isCompletedExceptionally,
-            "a synchronous sendAsync throw must complete the future exceptionally synchronously",
+            "a synchronous dispatch throw must complete the future exceptionally synchronously on return",
+        )
+        val ex = assertFailsWith<ExecutionException> { future.get(5, TimeUnit.SECONDS) }
+        assertEquals(
+            boom,
+            ex.cause,
+            "the dispatch throw must surface verbatim as the future's cause, was: ${ex.cause}",
         )
-        // 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 --------
@@ -870,4 +883,48 @@ class JdkHttpTransportTest {
 
         override fun canHandle(challenges: List<org.dexpace.sdk.core.http.auth.AuthenticateChallenge>): Boolean = false
     }
+
+    /**
+     * A minimal [HttpClient] whose async dispatch throws synchronously on the caller's thread,
+     * modelling a client (or a future JDK) that does not package every `sendAsync` failure into the
+     * returned future. Only [sendAsync] is exercised by [JdkHttpTransport.executeAsync]; the rest
+     * are inert stubs that are never invoked on the dispatch path under test.
+     */
+    private class SyncThrowingDispatchClient(
+        private val failure: RuntimeException,
+    ) : HttpClient() {
+        override fun cookieHandler(): Optional<CookieHandler> = Optional.empty()
+
+        override fun connectTimeout(): Optional<Duration> = Optional.empty()
+
+        override fun followRedirects(): HttpClient.Redirect = HttpClient.Redirect.NEVER
+
+        override fun proxy(): Optional<ProxySelector> = Optional.empty()
+
+        override fun sslContext(): SSLContext = SSLContext.getDefault()
+
+        override fun sslParameters(): SSLParameters = SSLParameters()
+
+        override fun authenticator(): Optional<Authenticator> = Optional.empty()
+
+        override fun version(): HttpClient.Version = HttpClient.Version.HTTP_1_1
+
+        override fun executor(): Optional<Executor> = Optional.empty()
+
+        override fun <T> send(
+            request: HttpRequest,
+            responseBodyHandler: HttpResponse.BodyHandler<T>,
+        ): HttpResponse<T> = throw UnsupportedOperationException("synchronous send is not used by this test double")
+
+        override fun <T> sendAsync(
+            request: HttpRequest,
+            responseBodyHandler: HttpResponse.BodyHandler<T>,
+        ): CompletableFuture<HttpResponse<T>> = throw failure
+
+        override fun <T> sendAsync(
+            request: HttpRequest,
+            responseBodyHandler: HttpResponse.BodyHandler<T>,
+            pushPromiseHandler: HttpResponse.PushPromiseHandler<T>?,
+        ): CompletableFuture<HttpResponse<T>> = throw failure
+    }
 }