Skip to content

Preserve caller context at async request dispatch - #990

Open
sylvesterkaczmarek wants to merge 3 commits into
openai:mainfrom
sylvesterkaczmarek:fix-async-request-dispatch-context
Open

sylvesterkaczmarek wants to merge 3 commits into
openai:mainfrom
sylvesterkaczmarek:fix-async-request-dispatch-context

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #952.

Remove the unconditional common-pool hop between request preparation and asynchronous transport entry at all 295 dispatch sites. The configured transport can now capture the caller's context before scheduling HTTP work. Network I/O and stream callbacks retain their existing executors; no new executor, option, or dependency is introduced.

Regression coverage exercises typed and raw responses, the synchronous client's async view, configured and explicit stream callback executors, pending response futures, and synchronous/asynchronous transport failures.

Execution boundary

Synchronous setup inside HttpClient.executeAsync now runs on the request-preparation completion thread. Transports remain responsible for offloading blocking I/O. This does not add general context propagation across transport queues, retries, asynchronous credentials, or application future chains.

Validation

  • All six new regressions fail on unmodified 46bf1c1 and pass with this change.
  • SKIP_MOCK_TESTS=true ./gradlew :openai-java-core:test :openai-java-client-okhttp:test: 6,050 passed, 598 skipped.
  • Core and OkHttp Kotlin lint tasks and git diff --check pass.
  • Castiron budget passes at 2,492 of 3,000 lines, without changing the policy.

The generator source is not present here; the same substitution is applied consistently to the checked-in service implementations.

@sylvesterkaczmarek sylvesterkaczmarek changed the title placeholder Preserve caller context at async request dispatch Sep 10, 2026
@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team as a code owner September 10, 2026 09:57
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-10T10:04:24.258769Z 2a0e4f8 PR opened
🔒 Security Review ✅ Completed 2026-09-10T10:02:46.101427Z 2a0e4f8 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a0e4f8b80

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix-async-request-dispatch-context branch 2 times, most recently from 3aaa4d6 to 2de5263 Compare October 1, 2026 08:29

Copy link
Copy Markdown
Contributor Author

@codex review

@sylvesterkaczmarek

Copy link
Copy Markdown
Contributor Author

The queued-dispatcher context issue is addressed and the review thread is resolved. Current head is 0 behind main. Validation on JDK 21: AsyncRequestDispatchTest passes, RetryingHttpClientTest passes, and git diff --check is clean. Could a maintainer review the current head when convenient?

@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix-async-request-dispatch-context branch from c910ff5 to d170fd6 Compare October 1, 2026 09:07

This branch has not been deployed

No deployments
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.

Async internal plumbing bypasses both dispatcherExecutorService and streamHandlerExecutor, forcing a hop through ForkJoinPool.commonPool()

1 participant