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

Do not rethrow SmallRye Config ConfigValidationException in Quarkus ConfigException #41187

Merged

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Jun 13, 2024

Reverts #37543 and provides a different fix.

I think wrapping SmallRye Config ConfigValidationException in Quarkus ConfigException is misleading because of the extra exception and the added message, which does not represent the actual problem(s). ConfigValidationException already records multiple exceptions that can happen during Config setup to a single summary of problems. The added exception and message Failed to read configuration properties when that was not the problem confused me a bit :)

This also eliminates the config profile printing in the error from the original issue #36376. I think it does not add any value.

@radcortez radcortez requested a review from yrodiere June 13, 2024 10:26
@yrodiere
Copy link
Member

Thanks. A bit surprised you found a solution that easily since from what I remember it was super hard to get #37543 working, but 🤷

Any chance you can put the revert commit and your alternative solution in separate commits? I'm having trouble understanding what's what :/

Also since there's no automated test... we'll need to check what happens with the original reproduceer from #36376 . I'll do that later, as that involves switching branches on my local repo and rebuilding Quarkus and well... that takes time and I'm currently working on something else.

This comment has been minimized.

@radcortez
Copy link
Member Author

Thanks. A bit surprised you found a solution that easily since from what I remember it was super hard to get #37543 working, but 🤷

Maybe this comment confused everyone #36376 (comment)? I guess it would be a correct assumption for most things, but SmallRye Config is part of the core dependencies, so you can directly reference the exception.

Any chance you can put the revert commit and your alternative solution in separate commits? I'm having trouble understanding what's what :/

Sure. Done!

Also since there's no automated test... we'll need to check what happens with the original reproduceer from #36376 . I'll do that later, as that involves switching branches on my local repo and rebuilding Quarkus and well... that takes time and I'm currently working on something else.

I did try it with your reproducer. This is how it looks like:

[INFO] Running org.acme.GreetingResourceIT
2024-06-13 11:09:46,315 INFO  [org.tes.doc.DockerClientProviderStrategy] (build-14) Loaded org.testcontainers.dockerclient.UnixSocketClientProviderStrategy from ~/.testcontainers.properties, will try it first
2024-06-13 11:09:46,710 INFO  [org.tes.doc.DockerClientProviderStrategy] (build-14) Found Docker environment with local Unix socket (unix:///var/run/docker.sock)
2024-06-13 11:09:46,712 INFO  [org.tes.DockerClientFactory] (build-14) Docker host IP address is localhost
2024-06-13 11:09:46,740 INFO  [org.tes.DockerClientFactory] (build-14) Connected to docker:
  Server Version: 24.0.2
  API Version: 1.43
  Operating System: Docker Desktop
  Total Memory: 16010 MB
2024-06-13 11:09:46,753 INFO  [org.tes.ima.PullPolicy] (build-14) Image pull policy will be performed by: DefaultPullPolicy()
2024-06-13 11:09:46,757 INFO  [org.tes.uti.ImageNameSubstitutor] (build-14) Image name substitution will be performed by: DefaultImageNameSubstitutor (composite of 'ConfigurationFileImageNameSubstitutor' and 'PrefixingImageNameSubstitutor')
2024-06-13 11:09:46,762 INFO  [org.tes.DockerClientFactory] (build-14) Checking the system...
2024-06-13 11:09:46,763 INFO  [org.tes.DockerClientFactory] (build-14) ✔︎ Docker server version should be at least 1.6.0
2024-06-13 11:09:46,830 INFO  [tc.doc.io/postgres:14] (build-14) Creating container for image: docker.io/postgres:14
2024-06-13 11:09:47,316 INFO  [org.tes.uti.RegistryAuthLocator] (build-14) Credential helper/store (docker-credential-desktop) does not have credentials for docker.io
2024-06-13 11:09:47,445 INFO  [tc.doc.io/postgres:14] (build-14) Reusing container with ID: 57be00faba6e1fecd75fcc0be76743532d8e45c2829254dad94d7fcb60ef4d1d and hash: f915c881d2926ee5f0d87578e2cbd39fef89f562
2024-06-13 11:09:47,446 INFO  [tc.doc.io/postgres:14] (build-14) Reusing existing container (57be00faba6e1fecd75fcc0be76743532d8e45c2829254dad94d7fcb60ef4d1d) and not creating a new one
2024-06-13 11:09:47,687 INFO  [tc.doc.io/postgres:14] (build-14) Container docker.io/postgres:14 started in PT0.856765S
2024-06-13 11:09:47,688 INFO  [tc.doc.io/postgres:14] (build-14) Container is started (JDBC URL: jdbc:postgresql://localhost:58573/quarkus?loggerLevel=OFF)
2024-06-13 11:09:47,688 INFO  [io.qua.dev.pos.dep.PostgresqlDevServicesProcessor] (build-14) Dev Services for PostgreSQL started.
2024-06-13 11:09:47,689 INFO  [io.qua.dat.dep.dev.DevServicesDatasourceProcessor] (build-14) Dev Services for default datasource (postgresql) started - container ID is 57be00faba6e
2024-06-13 11:09:47,692 INFO  [io.qua.hib.orm.dep.dev.HibernateOrmDevServicesProcessor] (build-7) Setting quarkus.hibernate-orm.database.generation=drop-and-create to initialize Dev Services managed database
Executing "/Users/radcortez/.sdkman/candidates/java/17.0.5-tem/bin/java -Dquarkus.http.port=8081 -Dquarkus.http.ssl-port=8444 -Dtest.url=http://localhost:8081 -Dquarkus.log.file.path=/Users/radcortez/Code/open-source/quarkus-reproducers/quarkus-36376/target/quarkus.log -Dquarkus.log.file.enable=true -Dquarkus.log.category."io.quarkus".level=INFO -Dquarkus.profile=integrationtest -Dquarkus.datasource.password=quarkus -Dquarkus.datasource.devservices.reuse=true -Dquarkus.datasource.db-kind=postgresql -Dquarkus.datasource.jdbc.url=jdbc:postgresql://localhost:58573/quarkus?loggerLevel=OFF -Dquarkus.datasource.username=quarkus -Dquarkus.hibernate-orm.database.generation=drop-and-create -jar /Users/radcortez/Code/open-source/quarkus-reproducers/quarkus-36376/target/quarkus-app/quarkus-run.jar"
Jun 13, 2024 11:09:49 AM org.hibernate.Version
INFO: HHH000412: Hibernate ORM core version 6.0.6.Final
Configuration validation failed:
	java.util.NoSuchElementException: SRCFG00011: Could not expand value some-unresolved-property in property hello.message
Log file '/Users/radcortez/Code/open-source/quarkus-reproducers/quarkus-36376/target/quarkus.log' was not created. Check if the options quarkus.log.level and quarkus.log.file.level are at least INFO (or more verbose).
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 65.93 s <<< FAILURE! - in org.acme.GreetingResourceIT
[ERROR] org.acme.GreetingResourceIT.testHelloEndpoint  Time elapsed: 0.013 s  <<< ERROR!
java.lang.RuntimeException: java.lang.IllegalStateException: Unable to determine the status of the running process. See the above logs for details
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.throwBootFailureException(QuarkusIntegrationTestExtension.java:373)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeEach(QuarkusIntegrationTestExtension.java:117)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.IllegalStateException: Unable to determine the status of the running process. See the above logs for details
	at io.quarkus.test.common.LauncherUtil.waitForCapturedListeningData(LauncherUtil.java:102)
	at io.quarkus.test.common.DefaultJarLauncher.start(DefaultJarLauncher.java:72)
	at io.quarkus.test.junit.IntegrationTestUtil.startLauncher(IntegrationTestUtil.java:194)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.doProcessStart(QuarkusIntegrationTestExtension.java:301)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.ensureStarted(QuarkusIntegrationTestExtension.java:169)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeAll(QuarkusIntegrationTestExtension.java:130)
	... 1 more

When running directly (with another example):

java -jar target/quarkus-app/quarkus-run.jar
Configuration validation failed:
	java.util.NoSuchElementException: SRCFG00011: Could not expand value missing in property my-config.expand
	SRCFG00050: my-config.unmapped in PropertiesConfigSource[source=jar:file:///Users/radcortez/Code/open-source/quarkus-reproducers/quarkus-39090/target/quarkus-app/app/quarkus-issue-39090-1.0.0-SNAPSHOT.jar!/application.properties] does not map to any root
	SRCFG00050: my-config.another in PropertiesConfigSource[source=jar:file:///Users/radcortez/Code/open-source/quarkus-reproducers/quarkus-39090/target/quarkus-app/app/quarkus-issue-39090-1.0.0-SNAPSHOT.jar!/application.properties] does not map to any root

@radcortez radcortez force-pushed the configuration-validation-exception branch from 40d0cab to 3d2282e Compare June 13, 2024 13:42

This comment has been minimized.

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM, though not to CI: there seem to be somewhat related errors (getCause called on null).

@radcortez radcortez force-pushed the configuration-validation-exception branch from 3d2282e to cbfde5e Compare June 14, 2024 11:13
@yrodiere yrodiere added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 14, 2024
Copy link

quarkus-bot bot commented Jun 14, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit cbfde5e.

✅ 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 can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/reactive-messaging-kafka

io.quarkus.it.kafka.KafkaConnectorTest.testFruits - History

  • Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.it.kafka.KafkaConnectorTest.testFruits(KafkaConnectorTest.java:63)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

@radcortez radcortez added triage/flaky-test and removed triage/waiting-for-ci Ready to merge when CI successfully finishes triage/flaky-test labels Jun 14, 2024
@radcortez radcortez merged commit 6cad08e into quarkusio:main Jun 14, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jun 14, 2024
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.

2 participants