Skip to content

fix(async): propagate cancellation through service futures#759

Open
goutamadwant wants to merge 1 commit into
openai:mainfrom
goutamadwant:fix/openai-java-654-async-cancellation
Open

fix(async): propagate cancellation through service futures#759
goutamadwant wants to merge 1 commit into
openai:mainfrom
goutamadwant:fix/openai-java-654-async-cancellation

Conversation

@goutamadwant

Copy link
Copy Markdown

Fixes #654.

Summary

  • Add internal CompletableFuture helpers that propagate cancellation to upstream and inner futures.
  • Use those helpers through async service futures so cancelling a parsed service future can cancel the raw response and HTTP futures.
  • Preserve cancellation through the default logging and retrying HTTP client wrappers.
  • Add a regression for cancelling EmbeddingServiceAsync.create() through the default client stack.

Tests

  • ./scripts/lint
  • ./scripts/test
  • ./gradlew :openai-java-core:test --tests com.openai.core.FuturesTest --tests com.openai.services.async.EmbeddingServiceAsyncTest.create_whenFutureCancelled_cancelsHttpFuture
  • ./gradlew :openai-java-client-okhttp:test --tests com.openai.client.okhttp.OkHttpClientTest.executeAsync_whenFutureCancelled_cancelsUnderlyingCall

@goutamadwant

Copy link
Copy Markdown
Author

Just a note on the size of the diff.

Most of the changed files are generated async service implementations. They all use the same future chain: prepare the request, call executeAsync, then parse the response. The cancellation bug is in that shared async composition pattern, so changing only one service would fix the regression for one endpoint but leave the same behavior in the rest of the async SDK.

This patch adds internal future helpers in core and applies them consistently across the generated async services. I also updated the default logging and retrying HTTP wrappers because they add their own CompletableFuture stages and would otherwise still break cancellation propagation before it reaches the underlying HTTP future.

No public cancellation API was added. The change keeps the existing API shape and makes cancellation propagate through the existing future chain.

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.

Cancelling the completable future of an OpenAI call doesn't cancel the underlying OkHttp request

1 participant