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.

Sign up for GitHub

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

Jump to bottom

Improve exception handling when application fails to start #42270

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −0
Draft

Improve exception handling when application fails to start #42270

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/runtime/src/main/java/io/quarkus/runtime/ApplicationLifecycleManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Copy link
Contributor Author

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 IOExceptions) in https:/smallrye/smallrye-config/blob/main/implementation/src/main/java/io/smallrye/config/AbstractLocationConfigSourceLoader.java to handle #42084

WDYT?

Copy link
Member

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 or ConfigValidationException?

Maybe by checking

quarkus/core/runtime/src/main/java/io/quarkus/runtime/configuration/QuarkusConfigFactory.java

Line 13 in 133cbd9

private static volatile SmallRyeConfig config;
, but there is no public API for that.

If not, I am thinking that we will need to patch SmallryeConfig to catch more exceptions (currently looking for IOExceptions) in https:/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.

Copy link
Contributor Author

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).

Copy link
Member

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?

Copy link
Member

Choose a reason for hiding this comment

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

Probably only for Mandrel?

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

@zakkak zakkak Aug 27, 2024 •
edited
Loading

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)

System.err.println("Failed to start application");
e.printStackTrace();
} else {
applicationLogger.errorv(e, "Failed to start application");
ensureConsoleLogsDrained();
Expand Down
Loading