Skip to content

Commit

Permalink
fix: catch exceptions from detach calls (#20656) (#20699)
Browse files Browse the repository at this point in the history
* fix: catch exceptions from detach calls

* add test

---------

Co-authored-by: Teppo Kurki <[email protected]>
Co-authored-by: Marco Collovati <[email protected]>
  • Loading branch information
3 people authored Dec 13, 2024
1 parent 17604bf commit ade092e
Showing 3 changed files with 88 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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<DetachEvent> 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'. "
Original file line number Diff line number Diff line change
@@ -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<Throwable> expectedExceptions = new HashSet<>();
Set<Throwable> 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();

0 comments on commit ade092e

Please sign in to comment.