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

Fix exception handling when a message consumer is blocking #12616

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

cescoffier
Copy link
Member

@cescoffier cescoffier commented Oct 9, 2020

When a message consumer fails and uses blocking=true, the caught exception must be captured and sent to the sender as reply exception.

@cescoffier
Copy link
Member Author

CC @tsegismont

@cescoffier cescoffier added this to the 1.10 - master milestone Oct 9, 2020
@cescoffier
Copy link
Member Author

@gsmet Marked this PR as backporting (after approval and merge of course), as it's clearly a bug. The fix should not conflict with anything else (famous last words)

@mkouba
Copy link
Contributor

mkouba commented Oct 9, 2020

@cescoffier Could you pls elaborate a bit about why is this fix needed?

@mkouba
Copy link
Contributor

mkouba commented Oct 9, 2020

BTW we should probably update our generated code to use io.vertx.core.Promise instead of io.vertx.core.Future...

@cescoffier
Copy link
Member Author

When failing the future, as done before, the sender do not get the exception (the vertx global exception handler does). With this it gets a failed reply and can react.

@tsegismont
Copy link
Contributor

@mkouba the related issue is #12599

Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @cescoffier

I trust that the body of verifyFailure is the same as in MessageConsumerFailureBlockingTest. Perhaps to better show that there shouldn't be any difference between blocking/non-blocking the two tests could be merged? That's up to you I'm fine with the PR.

@cescoffier
Copy link
Member Author

Yeah, I should be able to reuse the same method, let me fix this.

…ception must be captured and sent to the sender as reply exception.
@cescoffier
Copy link
Member Author

@tsegismont Fixed, I merged both tests.

@gsmet
Copy link
Member

gsmet commented Oct 9, 2020

I cancelled CI as I need the slots for an emergency release, will restart them later.

Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

Ah, now I finally understand! Thanks for the fix.

@gastaldi gastaldi merged commit b543252 into quarkusio:master Oct 9, 2020
@gsmet gsmet modified the milestones: 1.10 - master, 1.9.0.Final Oct 10, 2020
@cescoffier cescoffier deleted the fix-12599 branch August 1, 2021 07:32
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.

Error handling does not work with blocking event bus consumers
5 participants