Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArC: document the @ObservesAsync pitfall with reactive programming #43856

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Oct 14, 2024

Fixes #29191

@Ladicek Ladicek added the area/arc Issue related to ARC (dependency injection) label Oct 14, 2024
@Ladicek Ladicek requested review from mkouba and manovotn October 14, 2024 10:33
@Ladicek
Copy link
Contributor Author

Ladicek commented Oct 14, 2024

I have marked this PR as fixing #29191, because it looks like we're not keen on actually doing something out of spec here. If someone disagrees, please let me know and I will unmark this PR and keep the issue open.

Copy link

github-actions bot commented Oct 14, 2024

🙈 The PR is closed and the preview is expired.

Copy link

quarkus-bot bot commented Oct 14, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 439ab1d.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You may declare an observer method that has a return type of `CompletionStage<>` or `Uni<>`, but neither the return type nor the actual return value affect the result of `Event.fireAsync()`.
Further, if the observer declares a return type of `Uni<>`, the returned `Uni` will not be subscribed to, so it is quite possible that part of the observer logic will not even execute.

Therefore, it is recommended that observer methods, both synchronous and asynchronous, are always declared `void`.
Copy link
Member

Choose a reason for hiding this comment

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

@sheilamjones not sure who to report this to but this rule needs tweaking. AFAICS it is triggered by asynchronous.

@Ladicek Ladicek merged commit 5072d79 into quarkusio:main Oct 15, 2024
5 checks passed
@Ladicek Ladicek deleted the arc-document-async-observer-pitfall branch October 15, 2024 07:53
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subscribing to CompletionStage returned by Event.fireAsync doesn't seem to work correctly
4 participants