fix: wrap UI closing to catch exceptions (#20645)

When session destroy is triggered, wraps each UI close/detach to catch exceptions, making sure session destroy listeners are called. Fixes #20293
vaadin · Dec 9, 2024 · c194e71 · c194e71
1 parent e988d48
commit c194e71
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 12 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
@@ -702,18 +702,22 @@ public void fireSessionDestroy(VaadinSession vaadinSession) {
             }
             List<UI> uis = new ArrayList<>(session.getUIs());
             for (final UI ui : uis) {
-                ui.accessSynchronously(() -> {
-                    /*
-                     * close() called here for consistency so that it is always
-                     * called before a UI is removed. UI.isClosing() is thus
-                     * always true in UI.detach() and associated detach
-                     * listeners.
-                     */
-                    if (!ui.isClosing()) {
-                        ui.close();
-                    }
-                    session.removeUI(ui);
-                });
+                try {
+                    ui.accessSynchronously(() -> {
+                        /*
+                         * close() called here for consistency so that it is
+                         * always called before a UI is removed. UI.isClosing()
+                         * is thus always true in UI.detach() and associated
+                         * detach listeners.
+                         */
+                        if (!ui.isClosing()) {
+                            ui.close();
+                        }
+                        session.removeUI(ui);
+                    });
+                } catch (Exception e) {
+                    session.getErrorHandler().error(new ErrorEvent(e));
+                }
             }
             SessionDestroyEvent event = new SessionDestroyEvent(
                     VaadinService.this, session);

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
@@ -15,6 +15,8 @@
  */
 package com.vaadin.flow.server;
 
+import com.vaadin.flow.component.DetachEvent;
+import com.vaadin.flow.component.UI;
 import com.vaadin.flow.i18n.DefaultI18NProvider;
 import com.vaadin.flow.i18n.I18NProvider;
 import net.jcip.annotations.NotThreadSafe;
@@ -24,12 +26,14 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
@@ -353,6 +357,64 @@ public void testSessionDestroyListenerCalled_whenAnotherListenerThrows()
                 listener.callCount);
     }
 
+    @Test
+    public void testSessionDestroyListenerCalled_andOtherUiDetachCalled_whenUiClosingThrows()
+            throws ServiceException {
+        VaadinService service = createService();
+
+        TestSessionDestroyListener listener = new TestSessionDestroyListener();
+
+        service.addSessionDestroyListener(listener);
+
+        final AtomicBoolean secondUiDetached = new AtomicBoolean();
+        List<UI> UIs = new ArrayList<>();
+        MockVaadinSession vaadinSession = new MockVaadinSession(service) {
+            @Override
+            public Collection<UI> getUIs() {
+                return UIs;
+            }
+        };
+        vaadinSession.lock();
+        UI throwingUI = new UI() {
+            @Override
+            protected void onDetach(DetachEvent detachEvent) {
+                throw new RuntimeException();
+            }
+
+            @Override
+            public VaadinSession getSession() {
+                return vaadinSession;
+            }
+        };
+
+        throwingUI.getInternals().setSession(vaadinSession);
+        UIs.add(throwingUI);
+
+        UI detachingUI = new UI() {
+            @Override
+            protected void onDetach(DetachEvent detachEvent) {
+                secondUiDetached.set(true);
+            }
+
+            @Override
+            public VaadinSession getSession() {
+                return vaadinSession;
+            }
+        };
+        detachingUI.getInternals().setSession(vaadinSession);
+        UIs.add(detachingUI);
+
+        vaadinSession.unlock();
+
+        service.fireSessionDestroy(vaadinSession);
+
+        Assert.assertTrue("Second UI detach not called properly",
+                secondUiDetached.get());
+
+        Assert.assertEquals("SessionDestroyListener not called exactly once", 1,
+                listener.callCount);
+    }
+
     @Test
     public void testServiceDestroyListenerCalled_whenAnotherListenerThrows()
             throws ServletException, ServiceException {