From ade092e84f9bddfdb5cca8d220b41421de444ea5 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Fri, 13 Dec 2024 15:20:30 +0100 Subject: [PATCH] fix: catch exceptions from detach calls (#20656) (#20699) * fix: catch exceptions from detach calls * add test --------- Co-authored-by: Teppo Kurki Co-authored-by: Marco Collovati --- .../vaadin/flow/component/ComponentUtil.java | 8 ++- .../vaadin/flow/component/DetachNotifier.java | 11 ++- .../vaadin/flow/component/ComponentTest.java | 71 +++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/component/ComponentUtil.java b/flow-server/src/main/java/com/vaadin/flow/component/ComponentUtil.java index c6f13ddb902..711ebb73c51 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/ComponentUtil.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/ComponentUtil.java @@ -48,6 +48,7 @@ import com.vaadin.flow.router.Route; import com.vaadin.flow.router.Router; import com.vaadin.flow.server.Attributes; +import com.vaadin.flow.server.ErrorEvent; import com.vaadin.flow.server.VaadinService; import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.shared.Registration; @@ -338,7 +339,12 @@ public static void onComponentDetach(Component component) { } DetachEvent detachEvent = new DetachEvent(component); - component.onDetach(detachEvent); + try { + component.onDetach(detachEvent); + } catch (RuntimeException e) { + VaadinSession.getCurrent().getErrorHandler() + .error(new ErrorEvent(e)); + } fireEvent(component, detachEvent); // inform component about onEnabledState if parent and child states diff --git a/flow-server/src/main/java/com/vaadin/flow/component/DetachNotifier.java b/flow-server/src/main/java/com/vaadin/flow/component/DetachNotifier.java index 28f0259dad1..238fcce09d1 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/DetachNotifier.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/DetachNotifier.java @@ -17,6 +17,8 @@ import java.io.Serializable; +import com.vaadin.flow.server.ErrorEvent; +import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.shared.Registration; /** @@ -38,7 +40,14 @@ default Registration addDetachListener( ComponentEventListener listener) { if (this instanceof Component) { return ComponentUtil.addListener((Component) this, - DetachEvent.class, listener); + DetachEvent.class, event -> { + try { + listener.onComponentEvent(event); + } catch (RuntimeException e) { + VaadinSession.getCurrent().getErrorHandler() + .error(new ErrorEvent(e)); + } + }); } else { throw new IllegalStateException(String.format( "The class '%s' doesn't extend '%s'. " diff --git a/flow-server/src/test/java/com/vaadin/flow/component/ComponentTest.java b/flow-server/src/test/java/com/vaadin/flow/component/ComponentTest.java index d7e083c0010..be48dc61cf0 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/ComponentTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/ComponentTest.java @@ -18,6 +18,7 @@ import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -737,6 +738,76 @@ public void testDetachListener_eventOrder_childFirst() { parent.assertDetachEvents(1); } + @Test + public void testDetach_failingListeners_allListenersInvokedAndExceptionHandled() { + Set expectedExceptions = new HashSet<>(); + Set handledExceptions = new HashSet<>(); + VaadinSession session = new AlwaysLockedVaadinSession( + new MockVaadinServletService()); + session.setErrorHandler( + event -> handledExceptions.add(event.getThrowable())); + VaadinSession.setCurrent(session); + try { + UI ui = new UI(); + TestComponentContainer parent = new TestComponentContainer() { + @Override + protected void onDetach(DetachEvent detachEvent) { + getDetachEvents().incrementAndGet(); + UnsupportedOperationException exception = new UnsupportedOperationException( + "ON-DETACH-1"); + expectedExceptions.add(exception); + throw exception; + } + }; + TestComponent child = new TestComponent() { + @Override + protected void onDetach(DetachEvent detachEvent) { + getDetachEvents().incrementAndGet(); + UnsupportedOperationException exception = new UnsupportedOperationException( + "ON-DETACH-2"); + expectedExceptions.add(exception); + throw exception; + } + }; + child.track(); + parent.track(); + + child.addDetachListener(event -> { + if (event.getSource() instanceof TracksAttachDetach track) { + track.getDetachEvents().incrementAndGet(); + } + UnsupportedOperationException exception = new UnsupportedOperationException( + "DETACH-LISTENER-1"); + expectedExceptions.add(exception); + throw exception; + }); + parent.addDetachListener(event -> { + if (event.getSource() instanceof TracksAttachDetach track) { + track.getDetachEvents().incrementAndGet(); + } + UnsupportedOperationException exception = new UnsupportedOperationException( + "DETACH-LISTENER-2"); + expectedExceptions.add(exception); + throw exception; + }); + + parent.add(child); + ui.add(parent); + + child.assertDetachEvents(0); + parent.assertDetachEvents(0); + + ui.remove(parent); + + child.assertDetachEvents(3); + parent.assertDetachEvents(3); + + Assert.assertEquals(expectedExceptions, handledExceptions); + } finally { + VaadinSession.setCurrent(null); + } + } + @Test public void testAttachDetach_elementMoved_bothEventsTriggered() { UI ui = new UI();