fix: service destroy listener exceptions (#20622) (#20636)

Related to #20577 Co-authored-by: Teppo Kurki <[email protected]>
vaadin · Dec 9, 2024 · c5e53e5 · c5e53e5
1 parent ae7cf90
commit c5e53e5
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 5 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java
@@ -2205,8 +2205,21 @@ public Registration addServiceDestroyListener(
      */
     public void destroy() {
         ServiceDestroyEvent event = new ServiceDestroyEvent(this);
-        serviceDestroyListeners
-                .forEach(listener -> listener.serviceDestroy(event));
+        RuntimeException exception = null;
+        for (ServiceDestroyListener listener : serviceDestroyListeners) {
+            try {
+                listener.serviceDestroy(event);
+            } catch (RuntimeException e) {
+                if (exception == null) {
+                    exception = e;
+                } else {
+                    e.addSuppressed(e);
+                }
+            }
+        }
+        if (exception != null) {
+            throw exception;
+        }
     }
 
     /**

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java b/flow-server/src/main/java/com/vaadin/flow/server/VaadinServlet.java
@@ -623,10 +623,13 @@ static URL getApplicationUrl(HttpServletRequest request)
     @Override
     public void destroy() {
         super.destroy();
-        if (getService() != null) {
-            getService().destroy();
+        try {
+            if (getService() != null) {
+                getService().destroy();
+            }
+        } finally {
+            isServletInitialized = false;
         }
-        isServletInitialized = false;
     }
 
     private VaadinServletContext initializeContext() {

diff --git a/flow-server/src/test/java/com/vaadin/flow/server/VaadinServiceTest.java b/flow-server/src/test/java/com/vaadin/flow/server/VaadinServiceTest.java
@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
@@ -63,6 +64,7 @@
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
 
 /**
  *
@@ -99,6 +101,34 @@ public void sessionDestroy(SessionDestroyEvent event) {
         }
     }
 
+    private class ThrowingSessionDestroyListener
+            implements SessionDestroyListener {
+
+        @Override
+        public void sessionDestroy(SessionDestroyEvent event) {
+            throw new RuntimeException();
+        }
+    }
+
+    private class TestServiceDestroyListener implements ServiceDestroyListener {
+
+        int callCount = 0;
+
+        @Override
+        public void serviceDestroy(ServiceDestroyEvent event) {
+            callCount++;
+        }
+    }
+
+    private class ThrowingServiceDestroyListener
+            implements ServiceDestroyListener {
+
+        @Override
+        public void serviceDestroy(ServiceDestroyEvent event) {
+            throw new RuntimeException();
+        }
+    }
+
     private String createCriticalNotification(String caption, String message,
             String details, String url) {
         return VaadinService.createCriticalNotificationJSON(caption, message,
@@ -298,6 +328,48 @@ public void testFireSessionDestroy()
                 1, listener.callCount);
     }
 
+    @Test
+    public void testSessionDestroyListenerCalled_whenAnotherListenerThrows()
+            throws ServiceException {
+        VaadinService service = createService();
+
+        ThrowingSessionDestroyListener throwingListener = new ThrowingSessionDestroyListener();
+        TestSessionDestroyListener listener = new TestSessionDestroyListener();
+
+        service.addSessionDestroyListener(throwingListener);
+        service.addSessionDestroyListener(listener);
+
+        final AtomicInteger errorCount = new AtomicInteger();
+        MockVaadinSession vaadinSession = new MockVaadinSession(service);
+        vaadinSession.lock();
+        vaadinSession.setErrorHandler(e -> errorCount.getAndIncrement());
+        vaadinSession.unlock();
+        service.fireSessionDestroy(vaadinSession);
+
+        Assert.assertEquals("ErrorHandler not called exactly once", 1,
+                errorCount.get());
+
+        Assert.assertEquals("SessionDestroyListener not called exactly once", 1,
+                listener.callCount);
+    }
+
+    @Test
+    public void testServiceDestroyListenerCalled_whenAnotherListenerThrows()
+            throws ServletException, ServiceException {
+        VaadinService service = createService();
+
+        ThrowingServiceDestroyListener throwingListener = new ThrowingServiceDestroyListener();
+        TestServiceDestroyListener listener = new TestServiceDestroyListener();
+
+        service.addServiceDestroyListener(throwingListener);
+        service.addServiceDestroyListener(listener);
+
+        assertThrows(RuntimeException.class, () -> service.destroy());
+
+        Assert.assertEquals("ServiceDestroyListener not called exactly once", 1,
+                listener.callCount);
+    }
+
     @Test
     public void captionIsSetToACriticalNotification() {
         String notification = createCriticalNotification("foobar", "message",