Skip to content

fix: deliver request-adaptation failures through executeAsync's future#119

Merged
OmarAlJarrah merged 2 commits into
mainfrom
fix/async-adaptation-failure-delivery
Jun 16, 2026
Merged

fix: deliver request-adaptation failures through executeAsync's future#119
OmarAlJarrah merged 2 commits into
mainfrom
fix/async-adaptation-failure-delivery

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

Both transports adapt the request synchronously at the top of executeAsync, before returning the CompletableFuture<Response>:

override fun executeAsync(request: Request): CompletableFuture<Response> {
    val okRequest = requestAdapter.adapt(request)   // may throw
    ...
}

A failure raised while adapting the request was therefore thrown on the caller's thread, not delivered through the returned future. A caller composing on the future never observes it:

transport.executeAsync(req)
    .exceptionally(t -> recover(t));   // never runs — the exception is thrown at the call site

Reachable adaptation failures today:

  • JDKIllegalArgumentException for a CONNECT request (the JDK client reserves CONNECT for tunnelling).
  • OkHttpIllegalArgumentException from Request.Builder.method for a method/body mismatch (e.g. a body on a GET).

Both methods already document that they "complete exceptionally with the transport failure on error", so the synchronous throw violated their own contract.

Change

executeAsync now catches an adaptation failure and returns a future completed exceptionally, so every post-call error flows through one channel. The synchronous execute path is unchanged — it keeps throwing, honouring its @Throws(IOException) contract.

The OkHttp module targets Java 8, where CompletableFuture.failedFuture (JDK 9+) is unavailable, so it completes a fresh future manually via a small failedFuture helper; the JDK module (Java 11) uses CompletableFuture.failedFuture.

Tests

  • OkHttp: a body-bearing GET (which OkHttp rejects during adaptation) now yields a future that completes exceptionally with the IllegalArgumentException as its cause, instead of throwing synchronously.
  • JDK: a CONNECT request (rejected during adaptation) likewise surfaces through the future.

Both tests fail against the pre-change code (the synchronous throw escapes before the future is returned) and pass after.

Internal change to the transport modules only — no public API change.

Closes #118

Both transports adapted the request synchronously at the top of executeAsync, so a failure raised while adapting (a CONNECT request the JDK client rejects, or a method/body mismatch OkHttp rejects) was thrown on the caller's thread instead of completing the returned future exceptionally. A caller composing on the future with .exceptionally or .handle would never observe it.

executeAsync now catches adaptation failures and returns a future completed exceptionally, matching its documented "completes exceptionally on error" contract and giving async callers a single error channel. The OkHttp module targets Java 8, so it completes the future manually rather than using CompletableFuture.failedFuture (JDK 9+).

Closes #118
…ilures

In executeAsync, request adaptation runs synchronously on the caller's thread before the future is returned. Catching Exception (rather than Throwable) routes every recoverable adaptation failure into the future while letting JVM-fatal Errors (OutOfMemoryError, StackOverflowError) propagate up the caller's stack, where they belong, instead of packaging them into a future that may never be awaited.

The OkHttp onResponse callback intentionally keeps catching Throwable: it runs on a dispatcher thread where an escaped Throwable would strand the future uncompleted, so there it is the only way to guarantee completion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

executeAsync throws request-adaptation failures synchronously instead of via the returned future

1 participant