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

Improve error message for Reactive Messaging coroutine support #25654

Merged
merged 1 commit into from
May 18, 2022

Conversation

geoand
Copy link
Contributor

@geoand geoand commented May 18, 2022

Relates to: #25502

Copy link
Contributor

@ozangunalp ozangunalp left a comment

Choose a reason for hiding this comment

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

See comment

@@ -13,7 +13,7 @@ abstract class AbstractSubscribingCoroutineInvoker(private val beanInstance: Any
override fun invoke(vararg args: Any?): CompletableFuture<Any?> {
val coroutineScope = Arc.container().instance(ApplicationCoroutineScope::class.java).get()
val dispatcher: CoroutineDispatcher = Vertx.currentContext()?.let(::VertxDispatcher)
?: throw IllegalStateException("No Vertx context found")
?: throw IllegalStateException("No Vertx context found. Consider using @NonBlocking on the suspend method.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This change needs to be on io.quarkus.scheduler.kotlin.runtime.AbstractCoroutineInvoker
For reactive messaging, this is an edge case, I propose No Vertx context found. Consider emitting items on Vert.x context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you are right, wrong class!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually... the error is coming from AbstractSubscribingCoroutineInvoker, not io.quarkus.scheduler.kotlin.runtime.AbstractCoroutineInvoker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In any case, error message updated.

FWIW, this error should never happen for the scheduler, as suspend methods are always executed on a vert.x thread

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, ok I'd missed that. That's because there is an emitter used inside, which itself ends up calling an incoming method with suspend... But @NonBlocking won't solve anything on the case with Mutiny tick.

Shall we write something generic along :
"Consider using @NonBlocking on the caller method, or make sure the upstream emits items on the Vert.x context".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍🏼 , done

@geoand geoand requested a review from gastaldi May 18, 2022 15:24
@gastaldi gastaldi merged commit e3e1cc7 into quarkusio:main May 18, 2022
@quarkus-bot quarkus-bot bot added this to the 2.10 - main milestone May 18, 2022
@gastaldi gastaldi deleted the #25502 branch May 18, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants