Skip to content

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

@OmarAlJarrah

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    sdk-coresdk-core toolkittech-debtsimplification / cleanup

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions