Skip to content

[MNG-8727] fix unnecessary throws leftover; promoting ExecutorException to Exception being stringent to ensure catch #2336

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

Closed
Pankraz76 wants to merge 1 commit into from
Closed

[MNG-8727] fix unnecessary throws leftover; promoting ExecutorException to Exception being stringent to ensure catch #2336

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion impl/maven-executor/src/main/java/org/apache/maven/api/cli/ExecutorException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
* @since 4.0.0
*/
@Experimental
public class ExecutorException extends RuntimeException {
Copy link
Contributor Author

@Pankraz76 Pankraz76 May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

sounds important enough, being worth to give dedication and segregation.

public class ExecutorException extends Exception {
/**
* Constructs a new {@code InvokerException} with the specified detail message.
*
Expand Down
6 changes: 3 additions & 3 deletions impl/maven-executor/src/main/java/org/apache/maven/api/cli/ExecutorRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ default Builder toBuilder() {
* also discovered by standard means.
*/
@Nonnull
static Builder mavenBuilder(@Nullable Path installationDirectory) {
static Builder mavenBuilder(@Nullable Path installationDirectory) throws ExecutorException {
return new Builder(
MVN,
null,
Expand Down Expand Up @@ -462,7 +462,7 @@ public String toString() {
}

@Nonnull
static Path discoverInstallationDirectory() {
static Path discoverInstallationDirectory() throws ExecutorException {
String mavenHome = System.getProperty("maven.home");
if (mavenHome == null) {
throw new ExecutorException("requires maven.home Java System Property set");
Expand All @@ -471,7 +471,7 @@ static Path discoverInstallationDirectory() {
}

@Nonnull
static Path discoverUserHomeDirectory() {
static Path discoverUserHomeDirectory() throws ExecutorException {
String userHome = System.getProperty("user.home");
if (userHome == null) {
throw new ExecutorException("requires user.home Java System Property set");
Expand Down
2 changes: 1 addition & 1 deletion impl/maven-executor/src/main/java/org/apache/maven/cling/executor/ExecutorHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ enum Mode {
* properly initialized request builder.
*/
@Nonnull
ExecutorRequest.Builder executorRequest();
ExecutorRequest.Builder executorRequest() throws ExecutorException;

/**
* Executes the request with preferred mode executor.
Expand Down
14 changes: 7 additions & 7 deletions ...xecutor/src/main/java/org/apache/maven/cling/executor/embedded/EmbeddedMavenExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ protected void disposeRuntimeCreatedRealms(Context context) {
}
}
} catch (Exception e) {
throw new ExecutorException("Failed to dispose runtime created realms", e);
throw new RuntimeException(new ExecutorException("Failed to dispose runtime created realms", e));
Copy link
Contributor

@hgschmie hgschmie May 20, 2025

Choose a reason for hiding this comment

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

this makes zero sense. First make the actual exception a checked exception, then wrap it into a (untyped) runtime exception.

Now there is no longer a ExecutorException thrown but a RuntimeException.

AI Slop.

}
}

Expand Down Expand Up @@ -285,7 +285,7 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) {
args.toArray(new String[0]), r.cwd().toString(), stdout, stderr
});
} catch (Exception e) {
throw new ExecutorException("Failed to execute", e);
throw new RuntimeException(new ExecutorException("Failed to execute", e));
}
});
} else {
Expand Down Expand Up @@ -313,7 +313,7 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) {
r.stdOut().orElse(null),
r.stdErr().orElse(null));
} catch (Exception e) {
throw new ExecutorException("Failed to execute", e);
throw new RuntimeException(new ExecutorException("Failed to execute", e));
}
});
}
Expand All @@ -328,7 +328,7 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) {
commands,
keepAlive);
} catch (Exception e) {
throw new ExecutorException("Failed to create executor", e);
throw new RuntimeException(new ExecutorException("Failed to create executor", e));
} finally {
Thread.currentThread().setContextClassLoader(originalClassLoader);
System.setProperties(originalProperties);
Expand Down Expand Up @@ -402,7 +402,7 @@ protected void doClose(Context context) throws ExecutorException {
}
}

protected void validate(ExecutorRequest executorRequest) throws ExecutorException {}
protected void validate(ExecutorRequest ignored) {}

protected URLClassLoader createMavenBootClassLoader(Path boot, List<URL> extraClasspath) {
ArrayList<URL> urls = new ArrayList<>(extraClasspath);
Expand All @@ -413,11 +413,11 @@ protected URLClassLoader createMavenBootClassLoader(Path boot, List<URL> extraCl
try {
urls.add(f.toUri().toURL());
} catch (MalformedURLException e) {
throw new ExecutorException("Failed to build classpath: " + f, e);
throw new RuntimeException(new ExecutorException("Failed to build classpath: " + f, e));
}
});
} catch (IOException e) {
throw new ExecutorException("Failed to build classpath: " + e, e);
throw new RuntimeException(new ExecutorException("Failed to build classpath: " + e, e));
}
if (urls.isEmpty()) {
throw new IllegalArgumentException("Invalid Maven home directory; boot is empty");
Expand Down
15 changes: 10 additions & 5 deletions impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/HelperImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ public HelperImpl(
@Nullable Path installationDirectory,
@Nullable Path userHomeDirectory,
Executor embedded,
Executor forked) {
Executor forked)
throws ExecutorException {
this.defaultMode = requireNonNull(defaultMode);
this.installationDirectory = installationDirectory != null
? ExecutorRequest.getCanonicalPath(installationDirectory)
Expand All @@ -68,7 +69,7 @@ public Mode getDefaultMode() {
}

@Override
public ExecutorRequest.Builder executorRequest() {
public ExecutorRequest.Builder executorRequest() throws ExecutorException {
return ExecutorRequest.mavenBuilder(installationDirectory).userHomeDirectory(userHomeDirectory);
}

Expand All @@ -80,12 +81,16 @@ public int execute(Mode mode, ExecutorRequest executorRequest) throws ExecutorEx
@Override
public String mavenVersion() {
return cache.computeIfAbsent("maven.version", k -> {
ExecutorRequest request = executorRequest().build();
return getExecutor(Mode.AUTO, request).mavenVersion(request);
try {
ExecutorRequest request = executorRequest().build();
return getExecutor(Mode.AUTO, request).mavenVersion(request);
} catch (ExecutorException e) {
throw new RuntimeException(e);
}
});
}

protected Executor getExecutor(Mode mode, ExecutorRequest request) throws ExecutorException {
protected Executor getExecutor(Mode mode, ExecutorRequest request) {
return switch (mode) {
case AUTO -> getExecutorByRequest(request);
case EMBEDDED -> executors.get(Mode.EMBEDDED);
Expand Down
2 changes: 1 addition & 1 deletion impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private ExecutorRequest.Builder mojo(ExecutorRequest.Builder builder, String moj
return builder.argument(TOOLBOX + mojo).argument("--quiet").argument("-DforceStdout");
}

private void doExecute(ExecutorRequest.Builder builder) {
private void doExecute(ExecutorRequest.Builder builder) throws ExecutorException {
ExecutorRequest request = builder.build();
int ec = helper.execute(request);
if (ec != 0) {
Expand Down
5 changes: 3 additions & 2 deletions ...aven-executor/src/test/java/org/apache/maven/cling/executor/MavenExecutorTestSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import org.apache.maven.api.annotations.Nullable;
import org.apache.maven.api.cli.Executor;
import org.apache.maven.api.cli.ExecutorException;
import org.apache.maven.api.cli.ExecutorRequest;
import org.apache.maven.cling.executor.embedded.EmbeddedMavenExecutor;
import org.apache.maven.cling.executor.forked.ForkedMavenExecutor;
Expand Down Expand Up @@ -323,11 +324,11 @@ protected String mavenVersion(ExecutorRequest request) throws Exception {
return createAndMemoizeExecutor().mavenVersion(request);
}

public static ExecutorRequest.Builder mvn3ExecutorRequestBuilder() {
public static ExecutorRequest.Builder mvn3ExecutorRequestBuilder() throws ExecutorException {
return ExecutorRequest.mavenBuilder(Paths.get(System.getProperty("maven3home")));
}

public static ExecutorRequest.Builder mvn4ExecutorRequestBuilder() {
public static ExecutorRequest.Builder mvn4ExecutorRequestBuilder() throws ExecutorException {
return ExecutorRequest.mavenBuilder(Paths.get(System.getProperty("maven4home")));
}

Expand Down
17 changes: 9 additions & 8 deletions impl/maven-executor/src/test/java/org/apache/maven/cling/executor/impl/ToolboxToolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.nio.file.Paths;
import java.util.Map;

import org.apache.maven.api.cli.ExecutorException;
import org.apache.maven.cling.executor.ExecutorHelper;
import org.apache.maven.cling.executor.MavenExecutorTestSupport;
import org.apache.maven.cling.executor.MimirInfuser;
Expand Down Expand Up @@ -81,7 +82,7 @@ void dump4(ExecutorHelper.Mode mode) throws Exception {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void version3(ExecutorHelper.Mode mode) {
void version3(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn3ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -94,7 +95,7 @@ void version3(ExecutorHelper.Mode mode) {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void version4(ExecutorHelper.Mode mode) {
void version4(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn4ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -107,7 +108,7 @@ void version4(ExecutorHelper.Mode mode) {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void localRepository3(ExecutorHelper.Mode mode) {
void localRepository3(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn3ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -123,7 +124,7 @@ void localRepository3(ExecutorHelper.Mode mode) {
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
@Disabled("disable temporarily so that we can get the debug statement")
void localRepository4(ExecutorHelper.Mode mode) {
void localRepository4(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn4ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -138,7 +139,7 @@ void localRepository4(ExecutorHelper.Mode mode) {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void artifactPath3(ExecutorHelper.Mode mode) {
void artifactPath3(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn3ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -157,7 +158,7 @@ void artifactPath3(ExecutorHelper.Mode mode) {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void artifactPath4(ExecutorHelper.Mode mode) {
void artifactPath4(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn4ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -176,7 +177,7 @@ void artifactPath4(ExecutorHelper.Mode mode) {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void metadataPath3(ExecutorHelper.Mode mode) {
void metadataPath3(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn3ExecutorRequestBuilder().build().installationDirectory(),
Expand All @@ -191,7 +192,7 @@ void metadataPath3(ExecutorHelper.Mode mode) {
@Timeout(15)
@ParameterizedTest
@EnumSource(ExecutorHelper.Mode.class)
void metadataPath4(ExecutorHelper.Mode mode) {
void metadataPath4(ExecutorHelper.Mode mode) throws ExecutorException {
ExecutorHelper helper = new HelperImpl(
mode,
mvn4ExecutorRequestBuilder().build().installationDirectory(),
Expand Down