SONAR-8093 Listener execution must not fail step execution

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java
index 854dc1013fe4e735286bcc0b3311b1ada76e3691..199d70efb6cb05e9da71f81f4ca9430405b6fd84 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java
@@ -53,7 +53,7 @@ public final class ComputationStepExecutor {
       allStepsExecuted = true;
     } finally {
       if (listener != null) {
-        listener.finished(allStepsExecuted);
+        executeListener(allStepsExecuted);
       }
     }
   }
@@ -66,6 +66,16 @@ public final class ComputationStepExecutor {
     }
   }
 
+  private void executeListener(boolean allStepsExecuted) {
+    try {
+      listener.finished(allStepsExecuted);
+    } catch (Throwable e) {
+      // any Throwable throws by the listener going up the stack might hide an Exception/Error thrown by the step and
+      // cause it be swallowed. We don't wan't that => we catch Throwable
+      LOGGER.error("Execution of listener failed", e);
+    }
+  }
+
   @FunctionalInterface
   public interface Listener {
     void finished(boolean allStepsExecuted);

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java
index 197cdb50ba4f4c4ea5b32134f1c65c91d4b09519..bd91cafd454ae387ce5b98a8b98c12702ca3a6a2 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java
@@ -31,6 +31,7 @@ import org.sonar.server.computation.task.ChangeLogLevel;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
+import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
@@ -105,8 +106,8 @@ public class ComputationStepExecutorTest {
 
   private List<String> execute_logs_end_timing_for_each_ComputationStep_called_when_(LoggerLevel level) {
     try (ChangeLogLevel executor = new ChangeLogLevel(ComputationStepExecutor.class, level);
-         ChangeLogLevel step1 = new ChangeLogLevel(computationStep1.getClass(), level);
-         ChangeLogLevel step2 = new ChangeLogLevel(computationStep2.getClass(), level)) {
+      ChangeLogLevel step1 = new ChangeLogLevel(computationStep1.getClass(), level);
+      ChangeLogLevel step2 = new ChangeLogLevel(computationStep2.getClass(), level)) {
       new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2))
         .execute();
 
@@ -139,7 +140,16 @@ public class ComputationStepExecutorTest {
       verify(listener).finished(false);
       verifyNoMoreInteractions(listener);
     }
+  }
+
+  @Test
+  public void execute_does_not_fail_if_listener_throws_Throwable() {
+    ComputationStepExecutor.Listener listener = mock(ComputationStepExecutor.Listener.class);
+    doThrow(new Error("Facking error thrown by Listener"))
+        .when(listener)
+        .finished(anyBoolean());
 
+    new ComputationStepExecutor(mockComputationSteps(computationStep1), listener).execute();
   }
 
   private static ComputationSteps mockComputationSteps(ComputationStep... computationSteps) {