Skip to content

Commit

Permalink
[MNG-8066] Default exception handler does not handle recursion (#1558)
Browse files Browse the repository at this point in the history
If there is a recursion in throwable causes, Maven will hang forever, instead to return.

---

https://issues.apache.org/jira/browse/MNG-8066
  • Loading branch information
cstamas authored Jun 6, 2024
1 parent df00ea5 commit 8650720
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
import java.net.ConnectException;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Set;

import org.apache.maven.lifecycle.LifecycleExecutionException;
import org.apache.maven.model.building.ModelProblem;
Expand Down Expand Up @@ -87,13 +90,13 @@
*/
@Component(role = ExceptionHandler.class)
public class DefaultExceptionHandler implements ExceptionHandler {

@Override
public ExceptionSummary handleException(Throwable exception) {
return handle("", exception);
}

private ExceptionSummary handle(String message, Throwable exception) {
String reference = getReference(exception);
String reference = getReference(Collections.newSetFromMap(new IdentityHashMap<>()), exception);

List<ExceptionSummary> children = null;

Expand Down Expand Up @@ -153,8 +156,11 @@ private ExceptionSummary handle(ModelProblem problem, String projectId) {
}
}

private String getReference(Throwable exception) {
private String getReference(Set<Throwable> dejaVu, Throwable exception) {
String reference = "";
if (!dejaVu.add(exception)) {
return reference;
}

if (exception != null) {
if (exception instanceof MojoExecutionException) {
Expand Down Expand Up @@ -186,14 +192,14 @@ private String getReference(Throwable exception) {
}

if (StringUtils.isEmpty(reference)) {
reference = getReference(cause);
reference = getReference(dejaVu, cause);
}

if (StringUtils.isEmpty(reference)) {
reference = exception.getClass().getSimpleName();
}
} else if (exception instanceof LifecycleExecutionException) {
reference = getReference(exception.getCause());
reference = getReference(dejaVu, exception.getCause());
} else if (isNoteworthyException(exception)) {
reference = exception.getClass().getSimpleName();
}
Expand Down Expand Up @@ -222,7 +228,8 @@ private boolean isNoteworthyException(Throwable exception) {
private String getMessage(String message, Throwable exception) {
String fullMessage = (message != null) ? message : "";

// To break out of possible endless loop when getCause returns "this"
// To break out of possible endless loop when getCause returns "this", or dejaVu for n-level recursion (n>1)
Set<Throwable> dejaVu = Collections.newSetFromMap(new IdentityHashMap<>());
for (Throwable t = exception; t != null && t != t.getCause(); t = t.getCause()) {
String exceptionMessage = t.getMessage();

Expand All @@ -246,6 +253,11 @@ private String getMessage(String message, Throwable exception) {
} else if (!fullMessage.contains(exceptionMessage)) {
fullMessage = join(fullMessage, exceptionMessage);
}

if (!dejaVu.add(t)) {
fullMessage = join(fullMessage, "[CIRCULAR REFERENCE]");
break;
}
}

return fullMessage.trim();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,20 @@ public synchronized Throwable getCause() {
String expectedReference = "http://cwiki.apache.org/confluence/display/MAVEN/PluginContainerException";
assertEquals(expectedReference, summary.getReference());
}

@Test
public void testHandleExceptionSelfReferencing() {
RuntimeException boom3 = new RuntimeException("BOOM3");
RuntimeException boom2 = new RuntimeException("BOOM2", boom3);
RuntimeException boom1 = new RuntimeException("BOOM1", boom2);
boom3.initCause(boom1);

DefaultExceptionHandler handler = new DefaultExceptionHandler();
ExceptionSummary summary = handler.handleException(boom1);

assertEquals("BOOM1: BOOM2: BOOM3: [CIRCULAR REFERENCE]", summary.getMessage());
assertEquals("", summary.getReference());
assertEquals(0, summary.getChildren().size());
assertEquals(boom1, summary.getException());
}
}

0 comments on commit 8650720

Please sign in to comment.