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

Prevent ContextNotActiveException during invalid config validation if resteasy-reactive module is present #31518

Merged
merged 1 commit into from Mar 2, 2023

Conversation

luca-bassoricci
Copy link
Contributor

@luca-bassoricci luca-bassoricci commented Mar 1, 2023

Check for an active RequestContext before accessing ResteasyReactiveLocaleResolver#currentVertxRequest to prevent ContextNotActiveException

I addes only 1 test in integration-tests\hibernate-validator-resteasy-reactive module because extensions\hibernate-validator\deployment doesn't use resteasy-reactive which is a mandatory module to raise the problem.

I'm also looking to reproduce the problem in our backend code with some @RestClients interaction but takes time.

Fixes #31434

@quarkus-bot quarkus-bot bot added the area/hibernate-validator Hibernate Validator label Mar 1, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 1, 2023

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not contain an issue number (use Fix #1234 in the description instead)

This message is automatically generated by a bot.

@luca-bassoricci luca-bassoricci changed the title Fixes #31434 Prevent ContextNotActiveException during invalid config validation if resteasy-reactive module is present Mar 1, 2023
@gsmet
Copy link
Member

gsmet commented Mar 2, 2023

I can't say if it's the right fix but given what you described in #31434, I think you are onto something that needs fixing.

/cc @geoand who can probably provide some useful guidance (@geoand see the issue for a description of the problem)

@geoand
Copy link
Contributor

geoand commented Mar 2, 2023

We seem to also have #31544 which attempts to fix the problem

@geoand
Copy link
Contributor

geoand commented Mar 2, 2023

I think the approach taken in this PR here makes more sense

@geoand geoand marked this pull request as ready for review March 2, 2023 10:54
@quarkus-bot

This comment has been minimized.

Check for an active RequestContext before accessing
ResteasyReactiveLocaleResolver#currentVertxRequest to prevent
ContextNotActiveException
@gsmet
Copy link
Member

gsmet commented Mar 2, 2023

@luca-bassoricci I force pushed to your branch to fix the formatting issue.

@luca-bassoricci
Copy link
Contributor Author

@luca-bassoricci I force pushed to your branch to fix the formatting issue.

Grazie :)
I ran the formatter:format from my command shell and I thought imports were ordered, but I was wrong.

@luca-bassoricci
Copy link
Contributor Author

I can't say if it's the right fix but given what you described in #31434, I think you are onto something that needs fixing.

/cc @geoand who can probably provide some useful guidance (@geoand see the issue for a description of the problem)

public String resolveCurrentTenantIdentifier() {
// Make sure that we're in a request
if (!Arc.container().requestContext().isActive()) {
return null;
}

I used this kind of solution because I found it was used elsewhere.

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 2, 2023

Failing Jobs - Building da65125

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17
✔️ JVM Tests - JDK 19

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/reactive-routes/deployment 
! Skipped: extensions/resteasy-reactive/quarkus-resteasy-reactive-jackson/deployment extensions/resteasy-reactive/quarkus-resteasy-reactive-jaxb/deployment extensions/resteasy-reactive/quarkus-resteasy-reactive-jsonb/deployment and 9 more

📦 extensions/reactive-routes/deployment

io.quarkus.vertx.web.context.DuplicatedContextTest.testThatRoutesAreCalledOnDuplicatedContext line 58 - More details - Source on GitHub

io.smallrye.mutiny.TimeoutException
	at io.smallrye.mutiny.operators.uni.UniBlockingAwait.await(UniBlockingAwait.java:64)
	at io.smallrye.mutiny.groups.UniAwait.atMost(UniAwait.java:65)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid config value prevents app to startup with invalid stacktrace instead of validation error message
3 participants