Failures in -Djarmode=tools do not consistently return a non zero exit

[mpalourdio]

Hello,

we are dealing with multiple Spring boot versions in our CI.

As we want to leverage jar extraction and CDS, we need to deal with multiple scenarios :

1. Spring Boot 3.3+ and jar non-executable : extract and CDS are OK
2. Spring Boot < 3.3 : extract (and so CDS) is not available. It correctly fails with an exit(1) and claims Unsupported jarmode 'tools' . See here
3. Spring Boot 3.3+ and jar executable : extract fails with XXXX.jar is not compatible; ensure jar file is valid and launch script is not enabled but when ran from CLI, the exit code is 0, although an exception is thrown.

3. is painful, because you need to catch the STD output to check and grep for an error. Something like this pseudo bash

java -Djarmode=tools -jar /opt/my.jar extract --destination /opt/extracted 2>&1`; if [[ $cmd =~ "Error" ]] || [[ $cmd  =~ "Unsupported" ]]; then whatever_you_want_to_do; fi

This PR handles 3 by moving the exception to a system exit code, which seems to be more in line with this feature which is mostly intended to run as CLI and could lead to cleaner error handling, ie

java -Djarmode=tools -jar /opt/my.jar extract --destination /opt/extracted ||  whatever_you_want_to_do

[mhalbritter]

Hello!

I see that there's a issue to fix. Ideally the CLI should return code 1 in every error case.

Thanks for the PR, but I don't like that approach. Putting a System.exit() somewhere deep in the command handling code feels wrong. It would be better to let the AbortException bubble up the stack and then catch it in org.springframework.boot.loader.jarmode.JarModeLauncher and org.springframework.boot.loader.launch.JarModeRunner and then call System.exit(1) there. That's also the place to check for the DISABLE_SYSTEM_EXIT system property.

Do you want to implement that in this PR or should I file a new issue tracking this?

[mpalourdio]

@mhalbritter thanks for feedback, I will give this a shot in this PR a little later

[mpalourdio]

@mhalbritter I have pushed some modifications. Does this look (a bit) at what you were expecting ?

Also, i can't catch AbortException, as it's private static final

[philwebb]

Thanks for the updated PR @mpalourdio. Looking in more detail I think we should pull up our exiting error reporting logic to a central location. That ends up being quite a large change and something we'll need to apply in 3.3 so I'm going to take care of it in #43435.

Thanks for reporting the problem and for your PR.