-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 exception handling when application fails to start #42270
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Prevent the usage of the logger when the application has not started regardless of the cause since it's most likely not configured yet. Closes quarkusio#42084
253b312
to
9091010
Compare
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✖ | JVM Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | JVM Tests - JDK 21 | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | JVM Tests - JDK 17 Windows | Build |
Failures | Logs | Raw logs | 🔍 |
Full information is available in the Build summary check run.
You can consult the Develocity build scans.
Failures
⚙️ JVM Tests - JDK 17 #
- Failing: extensions/flyway/deployment
! Skipped: integration-tests/flyway integration-tests/hibernate-orm-compatibility-5.6/mariadb integration-tests/hibernate-orm-compatibility-5.6/postgresql and 5 more
📦 extensions/flyway/deployment
✖ io.quarkus.flyway.test.FlywayExtensionRepairAtStartTest.testRepairUsingDevMode
line 46
- History - More details - Source on GitHub
org.awaitility.core.ConditionTimeoutException:
Assertion condition defined as a Lambda expression in io.quarkus.flyway.test.FlywayExtensionRepairAtStartTest
Expecting any element of:
[org.jboss.logmanager.ExtLogRecord@66c478e1,
org.jboss.logmanager.ExtLogRecord@11556a41,
org.jboss.logmanager.ExtLogRecord@5673548a,
org.jboss.logmanager.ExtLogRecord@77f95488,
org.jboss.logmanager.ExtLogRecord@323f39e6,
⚙️ JVM Tests - JDK 21 #
- Failing: extensions/flyway/deployment
! Skipped: integration-tests/flyway integration-tests/hibernate-orm-compatibility-5.6/mariadb integration-tests/hibernate-orm-compatibility-5.6/postgresql and 5 more
📦 extensions/flyway/deployment
✖ io.quarkus.flyway.test.FlywayExtensionRepairAtStartTest.testRepairUsingDevMode
line 46
- History - More details - Source on GitHub
org.awaitility.core.ConditionTimeoutException:
Assertion condition defined as a Lambda expression in io.quarkus.flyway.test.FlywayExtensionRepairAtStartTest
Expecting any element of:
[org.jboss.logmanager.ExtLogRecord@1ff444d1,
org.jboss.logmanager.ExtLogRecord@33e4a584,
org.jboss.logmanager.ExtLogRecord@156af654,
org.jboss.logmanager.ExtLogRecord@53d59dbf,
org.jboss.logmanager.ExtLogRecord@6e3a99d8,
⚙️ JVM Tests - JDK 17 Windows #
- Failing: extensions/flyway/deployment
! Skipped: integration-tests/flyway integration-tests/hibernate-orm-compatibility-5.6/mariadb integration-tests/hibernate-orm-compatibility-5.6/postgresql and 5 more
📦 extensions/flyway/deployment
✖ io.quarkus.flyway.test.FlywayExtensionRepairAtStartTest.testRepairUsingDevMode
line 46
- History - More details - Source on GitHub
org.awaitility.core.ConditionTimeoutException:
Assertion condition defined as a Lambda expression in io.quarkus.flyway.test.FlywayExtensionRepairAtStartTest
Expecting any element of:
[org.jboss.logmanager.ExtLogRecord@579abb47,
org.jboss.logmanager.ExtLogRecord@7f19290b,
org.jboss.logmanager.ExtLogRecord@919a797,
org.jboss.logmanager.ExtLogRecord@683da236,
org.jboss.logmanager.ExtLogRecord@6aff6cf5,
Flaky tests - Develocity
⚙️ JVM Tests - JDK 17
📦 extensions/smallrye-reactive-messaging/deployment
✖ io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector
- History
Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"]
-java.lang.AssertionError
java.lang.AssertionError:
Expecting actual:
["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
["-3", "-4", "-5", "-6"]
at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)
📦 integration-tests/opentelemetry
✖ io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections
- History
Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
-org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
at io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
⚙️ JVM Tests - JDK 21
📦 extensions/smallrye-reactive-messaging/deployment
✖ io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector
- History
Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"]
-java.lang.AssertionError
java.lang.AssertionError:
Expecting actual:
["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
["-3", "-4", "-5", "-6"]
at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)
📦 integration-tests/opentelemetry
✖ io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections
- History
Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
-org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds.
at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
at io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
The test failure is related because we don't have the log records to test anymore. |
@@ -201,6 +201,9 @@ public static void run(Application application, Class<? extends QuarkusApplicati | |||
} else if (rootCause instanceof PreventFurtherStepsException | |||
&& !StringUtil.isNullOrEmpty(rootCause.getMessage())) { | |||
System.err.println(rootCause.getMessage()); | |||
} else if (!currentApplication.isStarted()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking again at this I realize that this check doesn't make any sense and it will essentially always be true.
@radcortez is there any way I can check if the configuration generation was successful or not (assuming that logging will only work if it was successful) other than checking for ConfigurationException
or ConfigValidationException
?
If not, I am thinking that we will need to patch SmallryeConfig to catch more exceptions (currently looking for IOException
s) in https://github.com/smallrye/smallrye-config/blob/main/implementation/src/main/java/io/smallrye/config/AbstractLocationConfigSourceLoader.java to handle #42084
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radcortez is there any way I can check if the configuration generation was successful or not (assuming that logging will only work if it was successful) other than checking for
ConfigurationException
orConfigValidationException
?
Maybe by checking
quarkus/core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java
Line 13 in 133cbd9
private static volatile SmallRyeConfig config; |
If not, I am thinking that we will need to patch SmallryeConfig to catch more exceptions (currently looking for
IOException
s) in https://github.com/smallrye/smallrye-config/blob/main/implementation/src/main/java/io/smallrye/config/AbstractLocationConfigSourceLoader.java to handle #42084
We can certainly improve AbstractLocationConfigSourceLoader
, but from the SmallRyeConfig
side, we only provide ConfigValidationException
for validation errors that the user can fix. I'm not sure if should be wrapping IOException
.
Alternatively, we can probably add some sort of flag into the runtime-generated config class that can tell us if the config was started successfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried checking QuarkusConfigFactory
but that doesn't work either, since in native executables the config is always set (done at build time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, can you confirm the repro steps of #42084? I've tried the steps, but I couldn't reproduce it. Maybe I'm missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably only for Mandrel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radcortez I can no longer reproduce the silent failure on main
, but the exceptions I am getting are still hiding the actual root cause.
On linux I am getting:
Exception in thread "Shutdown thread" java.lang.NullPointerException
at io.quarkus.runtime.ApplicationLifecycleManager$ShutdownHookThread.run(ApplicationLifecycleManager.java:455)
at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:896)
at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:872)
which gets fixed in #42794. If you try building Quarkus with this fix you will be able to reproduce #42084 on linux. Unfortunately on MacOS I am getting an NCDFE which I still didn't have the time to fix.
What is the output of the following in your case?
./mvnw -Dnative -pl integration-tests/picocli-native -Dnative.surefire.skip -Dformat.skip -Dno-descriptor-tests clean verify -Dquarkus.native.additional-build-args=-H:ThrowMissingRegistrationErrors=
./integration-tests/picocli-native/target/quarkus-integration-test-picocli-native-999-SNAPSHOT-runner -h
If the build succeeds and the binary invocation exits without any messages then that's the reproducer. The desired behaviour is to fail with an error message (and ideally with a stacktrace as well) not silently.
If it prints the help message then something is wrong with the reproducer and I will need more info about your setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I do get the NCDFE: Could not initialize class io.smallrye.common.net.HostName
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radcortez I created #42810 to resolve the macOS issue but that requires graalvm/graalvm-community-jdk21u#9 being merged and released in the next Mandrel 23.1.x release to work.
Unfortunately with Mandrel 24.0.x and #42810 the issue is not reproducible on macOS (so you need to reproduce in linux)
Prevent the usage of the logger when the application has not started
regardless of the cause since it's most likely not configured yet.
Closes #42084