Skip to content

fix(streaming): settle responses when the callback executor rejects work - #416

Closed
sylvesterkaczmarek wants to merge 1 commit into
anthropics:mainfrom
sylvesterkaczmarek:fix/close-async-stream-on-executor-rejection
Closed

sylvesterkaczmarek wants to merge 1 commit into
anthropics:mainfrom
sylvesterkaczmarek:fix/close-async-stream-on-executor-rejection

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Summary

Fixes #415.

Observe the completion of the streaming callback stage, so a rejected executor settles onCompleteFuture() exceptionally and closes the unread response. Previously, the ignored dependent future held the rejection while the public completion future remained pending and the response stayed open.

Use executor-independent cleanup, following the existing AutoPagerAsync pattern. Preserve the original transport failure if its bookkeeping races with rejection, and do not replace an earlier explicit close. User callbacks are never moved onto a fallback thread.

Validation

Four new tests cover a shutdown default executor before/after response completion, a rejecting per-subscription executor, earlier transport failures, explicit close, exactly-once resource release and ordinary delivery on a real worker thread.

  • Two rejection regressions fail on unchanged main; existing-error/close controls pass.
  • AsyncStreamResponseTest and AutoPagerAsyncTest: 31 tests passed, no skips.
  • Core Kotlin formatting checks passed.
  • Core Detekt checks passed for 2,315 main-source files and 1,851 test files, with no findings.
  • git diff --check passed.

Tested with OpenJDK 21 on macOS arm64. Validation used one Gradle worker, in-process Kotlin compilation and a 10 GiB heap through local command-line flags. No build configuration changes.

The complete repository suite, Java 8 runtime, live API calls and other operating systems were not tested. When an executor refuses work, callbacks cannot run there; callers receive the failure through the completion future and the response is released. No public interfaces, dependencies or existing test expectations change.

Based on main at 6324b459fcde08738a255adcd88439b68851d512.

@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team as a code owner October 4, 2026 00:34
@tomer-ant

Copy link
Copy Markdown
Contributor

Closing along with #415. The reasoning is in #415 (comment).

@tomer-ant tomer-ant closed this Oct 14, 2026
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.

A rejected streaming executor leaves completion pending and the response open

2 participants