From 5dbe9b416e8ad7928228e9f42acf534fe399087a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 10 Sep 2026 06:41:01 +0000 Subject: [PATCH] [MNG-8678] Address review: improve isFatal Javadoc and add soft-failure test Follow-up to #13055. The isFatal Javadoc @return tag now explains why RuntimeException is treated as fatal on par with Error (both indicate an unexpected JVM or framework state). A new test pins the soft-failure path so a future change to isFatal cannot silently break --fail-at-end for ordinary plugin failures. --- .../concurrent/BuildPlanExecutor.java | 4 ++- .../concurrent/BuildPlanExecutorTest.java | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java index 85c05e97beea..f2c5d68254bf 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java +++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutor.java @@ -599,7 +599,9 @@ private void executeStep(BuildStep step) throws IOException, LifecycleExecutionE * them can only be seen by looking at the failures themselves. * * @param failures The failures collected for a single project - * @return {@code true} if the build must be halted + * @return {@code true} if the build must be halted; checked exceptions (ordinary plugin failures) are + * soft and allow the reactor to continue with other projects, while {@link RuntimeException}s + * and {@link Error}s indicate an unexpected JVM or framework state and halt the build */ private static boolean isFatal(List failures) { return failures.stream().anyMatch(t -> t instanceof RuntimeException || !(t instanceof Exception)); diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java index f34e8ddcb874..9ef77e95bf04 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/concurrent/BuildPlanExecutorTest.java @@ -93,6 +93,34 @@ void exceptionThrownByBuildStepIsRecordedAsBuildFailure() throws Exception { assertTrue(session.getResult().getBuildSummary(project) instanceof org.apache.maven.execution.BuildFailure); } + /** + * A checked exception thrown by a build step is a soft failure: the reactor must not be halted, and + * other projects must still be built when {@code REACTOR_FAIL_AT_END} is in effect. This pins the + * soft-failure path of {@code isFatal} so a future change does not silently break {@code --fail-at-end} + * for ordinary plugin failures. + */ + @Test + void checkedExceptionThrownByBuildStepDoesNotHaltReactor() throws Exception { + MavenProject project = newProject(); + MavenSession session = newSession(project); + session.getRequest().setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_AT_END); + + ReactorContext reactorContext = execute(session, project, event -> {}, plan -> { + BuildStep step = plan.step(project, "validate") + .orElseThrow(() -> new IllegalStateException("no validate step in the plan")); + // LifecycleExecutionException is a checked exception — the soft-failure case for isFatal. + step.exception = new LifecycleExecutionException("plugin failure"); + step.status.set(BuildStep.FAILED); + }); + + assertTrue( + session.getResult().getBuildSummary(project) instanceof org.apache.maven.execution.BuildFailure, + "the project must be recorded as a failure"); + assertTrue( + !reactorContext.getReactorBuildStatus().isHalted(), + "a checked exception must not halt the reactor when REACTOR_FAIL_AT_END is set"); + } + /** * A project can end up with more than one failure: when a build step fails, the matching after:* step is * still run for cleanup and may fail on its own. Those failures are reported through a wrapper exception,