Skip to content

Commit f21402d

Browse files
committed
Consistently return non-zero exit codes for jarmode failures
Update jar mode launchers to catch all exceptions and return a non-zero exit code. This refinement also allows us to consolidate the existing error reporting logic to a central locations. Modes that wish to report a simple error rather than a full stacktrace can throw the newly introduced `JarModeErrorException`. Fixes gh-43435
1 parent 589697a commit f21402d

File tree

18 files changed

+230
-80
lines changed

18 files changed

+230
-80
lines changed

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/ExtractCommand.java

+4-23
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343

4444
import org.springframework.boot.jarmode.tools.JarStructure.Entry;
4545
import org.springframework.boot.jarmode.tools.JarStructure.Entry.Type;
46-
import org.springframework.boot.jarmode.tools.Layers.LayersNotEnabledException;
46+
import org.springframework.boot.loader.jarmode.JarModeErrorException;
4747
import org.springframework.util.Assert;
4848
import org.springframework.util.StreamUtils;
4949
import org.springframework.util.StringUtils;
@@ -118,12 +118,6 @@ void run(PrintStream out, Map<Option, String> options, List<String> parameters)
118118
catch (IOException ex) {
119119
throw new UncheckedIOException(ex);
120120
}
121-
catch (LayersNotEnabledException ex) {
122-
printError(out, "Layers are not enabled");
123-
}
124-
catch (AbortException ex) {
125-
printError(out, ex.getMessage());
126-
}
127121
}
128122

129123
private static void checkDirectoryIsEmpty(Map<Option, String> options, File destination) {
@@ -134,11 +128,11 @@ private static void checkDirectoryIsEmpty(Map<Option, String> options, File dest
134128
return;
135129
}
136130
if (!destination.isDirectory()) {
137-
throw new AbortException(destination.getAbsoluteFile() + " already exists and is not a directory");
131+
throw new JarModeErrorException(destination.getAbsoluteFile() + " already exists and is not a directory");
138132
}
139133
File[] files = destination.listFiles();
140134
if (files != null && files.length > 0) {
141-
throw new AbortException(destination.getAbsoluteFile() + " already exists and is not empty");
135+
throw new JarModeErrorException(destination.getAbsoluteFile() + " already exists and is not empty");
142136
}
143137
}
144138

@@ -147,18 +141,13 @@ private void checkJarCompatibility() throws IOException {
147141
try (ZipInputStream stream = new ZipInputStream(new FileInputStream(file))) {
148142
ZipEntry entry = stream.getNextEntry();
149143
if (entry == null) {
150-
throw new AbortException(
144+
throw new JarModeErrorException(
151145
"File '%s' is not compatible; ensure jar file is valid and launch script is not enabled"
152146
.formatted(file));
153147
}
154148
}
155149
}
156150

157-
private void printError(PrintStream out, String message) {
158-
out.println("Error: " + message);
159-
out.println();
160-
}
161-
162151
private void extractLibraries(FileResolver fileResolver, JarStructure jarStructure, Map<Option, String> options)
163152
throws IOException {
164153
String librariesDirectory = getLibrariesDirectory(options);
@@ -494,12 +483,4 @@ private boolean shouldExtractLayer(String layer) {
494483

495484
}
496485

497-
private static final class AbortException extends RuntimeException {
498-
499-
AbortException(String message) {
500-
super(message);
501-
}
502-
503-
}
504-
505486
}

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/Layers.java

+3-10
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.util.Iterator;
2020
import java.util.zip.ZipEntry;
2121

22+
import org.springframework.boot.loader.jarmode.JarModeErrorException;
23+
2224
/**
2325
* Provides information about the jar layers.
2426
*
@@ -62,22 +64,13 @@ default String getLayer(ZipEntry entry) {
6264
* Return a {@link Layers} instance for the currently running application.
6365
* @param context the command context
6466
* @return a new layers instance
65-
* @throws LayersNotEnabledException if layers are not enabled
6667
*/
6768
static Layers get(Context context) {
6869
IndexedLayers indexedLayers = IndexedLayers.get(context);
6970
if (indexedLayers == null) {
70-
throw new LayersNotEnabledException();
71+
throw new JarModeErrorException("Layers are not enabled");
7172
}
7273
return indexedLayers;
7374
}
7475

75-
final class LayersNotEnabledException extends RuntimeException {
76-
77-
LayersNotEnabledException() {
78-
super("Layers not enabled: Failed to load layer index file");
79-
}
80-
81-
}
82-
8376
}

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/ListLayersCommand.java

+2-14
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
import java.util.List;
2121
import java.util.Map;
2222

23-
import org.springframework.boot.jarmode.tools.Layers.LayersNotEnabledException;
24-
2523
/**
2624
* The {@code 'list-layers'} tools command.
2725
*
@@ -38,22 +36,12 @@ class ListLayersCommand extends Command {
3836

3937
@Override
4038
void run(PrintStream out, Map<Option, String> options, List<String> parameters) {
41-
try {
42-
Layers layers = Layers.get(this.context);
43-
printLayers(out, layers);
44-
}
45-
catch (LayersNotEnabledException ex) {
46-
printError(out, "Layers are not enabled");
47-
}
39+
Layers layers = Layers.get(this.context);
40+
printLayers(out, layers);
4841
}
4942

5043
void printLayers(PrintStream out, Layers layers) {
5144
layers.forEach(out::println);
5245
}
5346

54-
private void printError(PrintStream out, String message) {
55-
out.println("Error: " + message);
56-
out.println();
57-
}
58-
5947
}

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractCommandTests.java

+15-10
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@
3232
import org.junit.jupiter.api.Nested;
3333
import org.junit.jupiter.api.Test;
3434

35+
import org.springframework.boot.loader.jarmode.JarModeErrorException;
36+
3537
import static org.assertj.core.api.Assertions.assertThat;
38+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
3639
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
3740

3841
/**
@@ -172,17 +175,18 @@ void failsOnIncompatibleJar() throws IOException {
172175
try (FileWriter writer = new FileWriter(file)) {
173176
writer.write("text");
174177
}
175-
TestPrintStream out = run(file);
176-
assertThat(out).contains("is not compatible; ensure jar file is valid and launch script is not enabled");
178+
assertThatExceptionOfType(JarModeErrorException.class).isThrownBy(() -> run(file))
179+
.withMessageContaining("is not compatible; ensure jar file is valid and launch script is not enabled");
177180
}
178181

179182
@Test
180183
void shouldFailIfDirectoryIsNotEmpty() throws IOException {
181184
File destination = file("out");
182185
Files.createDirectories(destination.toPath());
183186
Files.createFile(new File(destination, "file.txt").toPath());
184-
TestPrintStream out = run(ExtractCommandTests.this.archive, "--destination", destination.getAbsolutePath());
185-
assertThat(out).contains("already exists and is not empty");
187+
assertThatExceptionOfType(JarModeErrorException.class)
188+
.isThrownBy(() -> run(ExtractCommandTests.this.archive, "--destination", destination.getAbsolutePath()))
189+
.withMessageContaining("already exists and is not empty");
186190
}
187191

188192
@Test
@@ -266,10 +270,10 @@ void extractsOnlySelectedLayers() throws IOException {
266270
}
267271

268272
@Test
269-
void printErrorIfLayersAreNotEnabled() throws IOException {
273+
void failsIfLayersAreNotEnabled() throws IOException {
270274
File archive = createArchive();
271-
TestPrintStream out = run(archive, "--layers");
272-
assertThat(out).hasSameContentAsResource("ExtractCommand-printErrorIfLayersAreNotEnabled.txt");
275+
assertThatExceptionOfType(JarModeErrorException.class).isThrownBy(() -> run(archive, "--layers"))
276+
.withMessage("Layers are not enabled");
273277
}
274278

275279
}
@@ -318,10 +322,11 @@ void extract() throws IOException {
318322
}
319323

320324
@Test
321-
void printErrorIfLayersAreNotEnabled() throws IOException {
325+
void failsIfLayersAreNotEnabled() throws IOException {
322326
File archive = createArchive();
323-
TestPrintStream out = run(archive, "--launcher", "--layers");
324-
assertThat(out).hasSameContentAsResource("ExtractCommand-printErrorIfLayersAreNotEnabled.txt");
327+
assertThatExceptionOfType(JarModeErrorException.class)
328+
.isThrownBy(() -> run(archive, "--launcher", "--layers"))
329+
.withMessage("Layers are not enabled");
325330
}
326331

327332
@Test

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractLayersCommandTests.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,12 @@
4242
import org.mockito.Mock;
4343
import org.mockito.junit.jupiter.MockitoExtension;
4444

45+
import org.springframework.boot.loader.jarmode.JarModeErrorException;
4546
import org.springframework.core.io.ClassPathResource;
4647
import org.springframework.util.FileCopyUtils;
4748

4849
import static org.assertj.core.api.Assertions.assertThat;
50+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
4951
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
5052
import static org.mockito.BDDMockito.given;
5153

@@ -146,8 +148,9 @@ void runWithJarFileContainingNoEntriesFails() throws IOException {
146148
}
147149
given(this.context.getArchiveFile()).willReturn(file);
148150
try (TestPrintStream out = new TestPrintStream(this)) {
149-
this.command.run(out, Collections.emptyMap(), Collections.emptyList());
150-
assertThat(out).contains("is not compatible");
151+
assertThatExceptionOfType(JarModeErrorException.class)
152+
.isThrownBy(() -> this.command.run(out, Collections.emptyMap(), Collections.emptyList()))
153+
.withMessageContaining("is not compatible");
151154
}
152155
}
153156

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ListLayersCommandTests.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222

2323
import org.junit.jupiter.api.Test;
2424

25+
import org.springframework.boot.loader.jarmode.JarModeErrorException;
26+
2527
import static org.assertj.core.api.Assertions.assertThat;
28+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2629

2730
/**
2831
* Tests for {@link ListLayersCommand}.
@@ -39,9 +42,9 @@ void shouldListLayers() throws IOException {
3942
}
4043

4144
@Test
42-
void shouldPrintErrorWhenLayersAreNotEnabled() throws IOException {
43-
TestPrintStream out = run(createArchive());
44-
assertThat(out).hasSameContentAsResource("list-layers-output-layers-disabled.txt");
45+
void shouldFailWhenLayersAreNotEnabled() {
46+
assertThatExceptionOfType(JarModeErrorException.class).isThrownBy(() -> run(createArchive()))
47+
.withMessage("Layers are not enabled");
4548
}
4649

4750
private TestPrintStream run(File archive) {

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/resources/org/springframework/boot/jarmode/tools/ExtractCommand-printErrorIfLayersAreNotEnabled.txt

-2
This file was deleted.

spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/resources/org/springframework/boot/jarmode/tools/list-layers-output-layers-disabled.txt

-2
This file was deleted.

spring-boot-project/spring-boot-tools/spring-boot-loader-classic/src/main/java/org/springframework/boot/loader/jarmode/JarMode.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,7 +36,8 @@ public interface JarMode {
3636
* Run the jar in the given mode.
3737
* @param mode the mode to use
3838
* @param args any program arguments
39+
* @throws JarModeErrorException on an error that should print a simple error message
3940
*/
40-
void run(String mode, String[] args);
41+
void run(String mode, String[] args) throws JarModeErrorException;
4142

4243
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright 2012-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.loader.jarmode;
18+
19+
/**
20+
* Simple {@link RuntimeException} used to fail the jar mode with a simple printed error
21+
* message.
22+
*
23+
* @author Phillip Webb
24+
* @since 3.3.7
25+
*/
26+
public class JarModeErrorException extends RuntimeException {
27+
28+
public JarModeErrorException(String message) {
29+
super(message);
30+
}
31+
32+
public JarModeErrorException(String message, Throwable cause) {
33+
super(message, cause);
34+
}
35+
36+
}

spring-boot-project/spring-boot-tools/spring-boot-loader-classic/src/main/java/org/springframework/boot/loader/jarmode/JarModeLauncher.java

+31-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,11 +31,31 @@ public final class JarModeLauncher {
3131

3232
static final String DISABLE_SYSTEM_EXIT = JarModeLauncher.class.getName() + ".DISABLE_SYSTEM_EXIT";
3333

34+
static final String SUPPRESSED_SYSTEM_EXIT_CODE = JarModeLauncher.class.getName() + ".SUPPRESSED_SYSTEM_EXIT_CODE";
35+
3436
private JarModeLauncher() {
3537
}
3638

3739
public static void main(String[] args) {
3840
String mode = System.getProperty("jarmode");
41+
boolean disableSystemExit = Boolean.getBoolean(DISABLE_SYSTEM_EXIT);
42+
try {
43+
runJarMode(mode, args);
44+
if (disableSystemExit) {
45+
System.setProperty(SUPPRESSED_SYSTEM_EXIT_CODE, "0");
46+
}
47+
}
48+
catch (Throwable ex) {
49+
printError(ex);
50+
if (disableSystemExit) {
51+
System.setProperty(SUPPRESSED_SYSTEM_EXIT_CODE, "1");
52+
return;
53+
}
54+
System.exit(1);
55+
}
56+
}
57+
58+
private static void runJarMode(String mode, String[] args) {
3959
List<JarMode> candidates = SpringFactoriesLoader.loadFactories(JarMode.class,
4060
ClassUtils.getDefaultClassLoader());
4161
for (JarMode candidate : candidates) {
@@ -44,10 +64,17 @@ public static void main(String[] args) {
4464
return;
4565
}
4666
}
47-
System.err.println("Unsupported jarmode '" + mode + "'");
48-
if (!Boolean.getBoolean(DISABLE_SYSTEM_EXIT)) {
49-
System.exit(1);
67+
throw new JarModeErrorException("Unsupported jarmode '" + mode + "'");
68+
}
69+
70+
private static void printError(Throwable ex) {
71+
if (ex instanceof JarModeErrorException) {
72+
String message = ex.getMessage();
73+
System.err.println("Error: " + message);
74+
System.err.println();
75+
return;
5076
}
77+
ex.printStackTrace();
5178
}
5279

5380
}

spring-boot-project/spring-boot-tools/spring-boot-loader-classic/src/main/java/org/springframework/boot/loader/jarmode/TestJarMode.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -33,6 +33,12 @@ public boolean accepts(String mode) {
3333
@Override
3434
public void run(String mode, String[] args) {
3535
System.out.println("running in " + mode + " jar mode " + Arrays.asList(args));
36+
if (args.length > 0 && "error".equals(args[0])) {
37+
throw new JarModeErrorException("error message");
38+
}
39+
if (args.length > 0 && "fail".equals(args[0])) {
40+
throw new IllegalStateException("bad");
41+
}
3642
}
3743

3844
}

0 commit comments

Comments
 (0)