Skip to content

Decide whether executeAsync should catch Exception or RuntimeException for adaptation failures #122

Closed

Description

@OmarAlJarrah

Follow-up from #119.

executeAsync in both transports now wraps request adaptation in catch (e: Exception) and routes the failure into a failed future. The reachable adaptation failures are all RuntimeException (IllegalArgumentException: a CONNECT request on JDK, a body on a body-forbidden method on OkHttp), so the catch is broader than the documented cases.

The breadth is defensible — the intent is to route any adaptation failure through the future, and Error is deliberately excluded so JVM-fatal conditions still propagate. This issue is to make that an explicit decision rather than an incidental one:

  • Option A — keep Exception. Confirm the intent is "any non-fatal adaptation failure, including an adapter bug such as an NPE, surfaces via the future", and make the inline comment say so.
  • Option B — narrow to RuntimeException. Matches the actually-reachable failures and lets a checked exception from a future adapter step propagate per the sync contract instead.

Low priority; either outcome is fine. The point is to pick one deliberately and document it. Reasonable wontfix candidate if Option A is considered already settled.

Activity

  1. added 2 commits that reference this issue on Jun 17, 2026
    504976d
    c30dfd1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    sdk-coresdk-core toolkittech-debtsimplification / cleanup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions