Skip to content

Invert the unary path onto an async spine - #434

Draft
eseay wants to merge 1 commit into
eddie/with-deadline-helperfrom
eddie/async-unary-spine
Draft

eseay wants to merge 1 commit into
eddie/with-deadline-helperfrom
eddie/async-unary-spine

Conversation

@eseay

@eseay eseay commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #432 (targets eddie/with-deadline-helper) — this is what gives withDeadline its first caller.

What this does

ProtocolClient's async unary() was a wrapper around the callback version via UnaryAsyncWrapper. This flips that: async unary() is now the real implementation, and the callback version is a thin Task wrapper around it.

To do that without breaking anyone, Interceptor, UnaryInterceptor, and HTTPClientInterface each gain async versions of their methods as protocol requirements with default implementations that call through to the existing callback versions. No new public types, no adapters — every existing conformer still compiles and still gets called.

Key decision

Defaults only point one direction: async falls back to callback, never the reverse. HTTPClientInterface's callback unary() deliberately keeps no default, because giving it one would let both sides default to each other — a type implementing neither would loop forever with no compile error. That case is covered by a test that would hang, not fail, if this were ever violated.

Behavior changes

  • Canceling mid-request used to make an async caller hang forever (the completion was just never called). It now returns a .canceled response. The callback path keeps today's behavior for now.
  • A response synthesized after losing the deadline race no longer carries over headers/trailers from a canceled response. Both built-in HTTP clients already report those as empty in that case, so this is a no-op in practice.
  • Completion callbacks now run on the general concurrent executor instead of URLSession's serial delegate queue — worth knowing if anything relied on serial delivery.

Scope

Unary only. Streaming still uses the old callback chain and TimeoutTimer for now.

Testing

swift test (101/101), make testconformance (966/966 URLSession + 1681/1681 NIO), SwiftLint clean, iOS/macOS builds green. TimeoutTests.swift passes unmodified, which is the strongest signal the deadline bridge behaves correctly.

@eseay
eseay force-pushed the eddie/async-unary-spine branch from a397cae to 3548415 Compare August 8, 2026 19:42
ProtocolClient's async unary() was a wrapper around the closure-based
one via UnaryAsyncWrapper. This inverts that: async unary() is now the
real implementation, and the closure-based version is a thin Task
wrapper over it.

The async spellings are added directly to Interceptor, UnaryInterceptor,
and HTTPClientInterface as protocol requirements with default
implementations that bridge to the existing closure-based ones through
one shared helper, withSingleResume. No new public types, no adapters,
no InterceptorFactory changes -- every existing conformer of all three
protocols keeps compiling and keeps being invoked unchanged.

A defaulted spelling always bridges toward a spelling with no default,
never the reverse: HTTPClientInterface's closure-based unary() keeps no
default so the cycle can't close, and a type implementing neither
spelling terminates at a pass-through rather than looping. That
invariant has its own regression test.

withDeadline (from the prior commit) gets its first caller here,
replacing TimeoutTimer on the unary path; TimeoutTimer itself is
untouched since the stream path still uses it. UnaryAsyncWrapper and
its test are deleted.

Two intentional behavior changes, both strictly better than what they
replace:
- Canceling during interceptor execution used to leave an async caller
  hanging forever (the completion was simply never invoked, so
  UnaryAsyncWrapper's continuation never resumed). The async path now
  returns a `.canceled` ResponseMessage instead. The closure-based path
  keeps dropping the completion for now, with a TODO to fix it once the
  callback-based ProtocolClientInterface methods become default
  implementations.
- A response synthesized after losing the deadline race now always
  carries empty headers/trailers instead of copying them from a
  canceled response. Both built-in HTTP clients already report empty
  headers/trailers in that case, so this is a no-op in practice,
  confirmed by conformance.

Covered by 11 new tests across two files: interceptor chain ordering
(FIFO request / LIFO response), the closure<->async bridge in both
directions, the double-`proceed`/double-`onResponse` guards, and the
implements-neither-spelling termination case. TimeoutTests.swift, which
exercises withDeadline end-to-end through a callback-only transport
double, passes unmodified.

swift test: 102/102 across 21 suites. make testconformance: 966/966
(URLSession) + 1681/1681 (NIO). SwiftLint clean. xcodebuild green for
iOS Simulator and macOS.

Signed-off-by: Eddie Seay <eddie.seay@cfacorp.com>
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.

1 participant