Fix oauth2 blocking token fetch - #1650
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new supplyAsync call uses the default common pool for a blocking HTTP request instead of the application/workflow-managed executor, which can cause thread starvation and unmanaged thread usage under load.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses downstream reports of OAuth2 token retrieval causing blocking behavior by moving token validation/fetch onto an asynchronous CompletableFuture path in the HTTP executor/auth flow.
Changes:
- Offloads OAuth2 token fetch + JWT conversion/issuer validation to
CompletableFuture.supplyAsync(...). - Avoids eager creation of a completed future when no
AuthProvideris present by switching toorElseGet(...).
File summaries
| File | Description |
|---|---|
| impl/http/src/main/java/io/serverlessworkflow/impl/executors/http/auth/JaxRSAccessTokenProvider.java | Wrapes token fetch/validation in supplyAsync to avoid running blocking work inline. |
| impl/http/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractRequestExecutor.java | Minor Optional/CompletableFuture usage adjustment (orElseGet) in request execution chain. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2e94d1e to
d528261
Compare
Token validation and fetching were running synchronously on the
caller's thread, blocking it during the OAuth2 token request. Wrap
the work in CompletableFuture.supplyAsync using the workflow's
executorService so it runs off the calling thread.
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
d528261 to
39e4125
Compare
There was a problem hiding this comment.
🟡 Changes recommended
supplyAsync(..., executor) can still throw synchronously on task submission (e.g., rejected execution), which can reintroduce the synchronous-failure behavior this PR is aiming to avoid.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, localized, and aligns token fetching with the existing async executor usage pattern without introducing API or behavioral regressions beyond making the operation non-blocking for callers.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Special notes for reviewers:
Additional information (if needed):
This pull request tries to solve the following issue quarkiverse/quarkus-flow#923 in downstream project.