Suppress unavoidable UIDetachedException (#11146)

* Suppress unavoidable UIDetachedException

Fixes #11144

5 files changed, 145 insertions, 5 deletions

diff --git a/uitest/src/main/java/com/vaadin/tests/components/AbstractTestUI.java b/uitest/src/main/java/com/vaadin/tests/components/AbstractTestUI.java
index 7b10c1a7f3..b030382ca6 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/AbstractTestUI.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/AbstractTestUI.java
@@ -177,6 +177,11 @@ public abstract class AbstractTestUI extends UI {
         getLayout().addComponent(c);
     }
 
+    public void addComponent(Component c, String id) {
+        c.setId(id);
+        addComponent(c);
+    }
+
     public void addComponents(Component... c) {
         getLayout().addComponents(c);
     }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/ui/DetachedAccessErrorHandling.java b/uitest/src/main/java/com/vaadin/tests/components/ui/DetachedAccessErrorHandling.java
new file mode 100644
index 0000000000..7834f4cddc
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/ui/DetachedAccessErrorHandling.java
@@ -0,0 +1,78 @@
+package com.vaadin.tests.components.ui;
+
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.Constants;
+import com.vaadin.server.ErrorHandler;
+import com.vaadin.server.ErrorHandlingRunnable;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.UIDetachedException;
+
+@Widgetset(Constants.DEFAULT_WIDGETSET)
+public class DetachedAccessErrorHandling extends AbstractTestUI {
+
+    private static final Runnable NOP = () -> {
+    };
+
+    private static final class ListErrorHandler implements ErrorHandler {
+        private List<com.vaadin.server.ErrorEvent> errors = new CopyOnWriteArrayList<>();
+
+        @Override
+        public void error(com.vaadin.server.ErrorEvent event) {
+            errors.add(event);
+        }
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        ListErrorHandler errorHandler = ensureErrorHandlerSet();
+
+        addComponent(new Button("Show errors", event -> {
+            errorHandler.errors.forEach(error -> {
+                Label errorLabel = new Label(error.getThrowable().getMessage());
+                errorLabel.setStyleName("errorLabel");
+                addComponent(errorLabel);
+            });
+        }), "show");
+
+        addComponent(new Button("Add simple detach listener", event -> {
+            addDetachListener(detachEvent -> access(NOP));
+        }), "simple");
+
+        addComponent(new Button("Add error handling detach listener", event -> {
+            addDetachListener(detachEvent -> {
+                access(new ErrorHandlingRunnable() {
+                    @Override
+                    public void run() {
+                    }
+
+                    @Override
+                    public void handleError(Exception exception) {
+                        if (exception instanceof UIDetachedException) {
+                            UIDetachedException ignore = (UIDetachedException) exception;
+                        } else {
+                            throw new RuntimeException(exception);
+                        }
+                    }
+                });
+            });
+        }), "handling");
+    }
+
+    private ListErrorHandler ensureErrorHandlerSet() {
+        ErrorHandler currentErrorHandler = getSession().getErrorHandler();
+
+        if (!(currentErrorHandler instanceof ListErrorHandler)) {
+            currentErrorHandler = new ListErrorHandler();
+            getSession().setErrorHandler(currentErrorHandler);
+        }
+
+        return (ListErrorHandler) currentErrorHandler;
+    }
+
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java b/uitest/src/main/java/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java
index bb229aa13c..07b2a59958 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/ui/UIAccessExceptionHandling.java
@@ -6,6 +6,7 @@ import java.util.concurrent.Future;
 
 import com.vaadin.server.DefaultErrorHandler;
 import com.vaadin.server.ErrorHandler;
+import com.vaadin.server.ErrorHandlingRunnable;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.server.VaadinService;
 import com.vaadin.tests.components.AbstractTestUIWithLog;
@@ -70,6 +71,27 @@ public class UIAccessExceptionHandling extends AbstractTestUIWithLog
                     CurrentInstance.restoreInstances(instances);
                 }));
 
+        addComponent(
+                new Button("Throw through ErrorHandlingRunnable", event -> {
+                    access(new ErrorHandlingRunnable() {
+                        @Override
+                        public void run() {
+                            log.clear();
+                            throw new NullPointerException();
+                        }
+
+                        @Override
+                        public void handleError(Exception exception) {
+                            // "Handle" other exceptions, but leave NPE for
+                            // default handler
+                            if (exception instanceof NullPointerException) {
+                                NullPointerException npe = (NullPointerException) exception;
+                                throw npe;
+                            }
+                        }
+                    });
+                }));
+
         addComponent(new Button("Clear", event -> log.clear()));
     }
 
diff --git a/uitest/src/test/java/com/vaadin/tests/components/ui/DetachedAccessErrorHandlingTest.java b/uitest/src/test/java/com/vaadin/tests/components/ui/DetachedAccessErrorHandlingTest.java
new file mode 100644
index 0000000000..da9e9e3d74
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/ui/DetachedAccessErrorHandlingTest.java
@@ -0,0 +1,31 @@
+package com.vaadin.tests.components.ui;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class DetachedAccessErrorHandlingTest extends SingleBrowserTest {
+    @Test
+    public void testDetachedErrorHandling_pageOpen_noErrors() {
+        openTestURL();
+
+        $(ButtonElement.class).id("simple").click();
+        assertNoErrors();
+
+        // The thing to really test here is that nothing is logged to stderr,
+        // but that's not practical to detect
+        $(ButtonElement.class).id("handling").click();
+        assertNoErrors();
+    }
+
+    private void assertNoErrors() {
+        // Reload page to trigger detach event
+        openTestURL();
+
+        $(ButtonElement.class).id("show").click();
+        Assert.assertEquals(0, findElements(By.className("errorLabel")).size());
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/ui/UIAccessExceptionHandlingTest.java b/uitest/src/test/java/com/vaadin/tests/components/ui/UIAccessExceptionHandlingTest.java
index 8745f78e42..6a973cdd51 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/ui/UIAccessExceptionHandlingTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/ui/UIAccessExceptionHandlingTest.java
@@ -5,9 +5,9 @@ import static org.junit.Assert.assertEquals;
 import org.junit.Test;
 
 import com.vaadin.testbench.elements.ButtonElement;
-import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.SingleBrowserTest;
 
-public class UIAccessExceptionHandlingTest extends MultiBrowserTest {
+public class UIAccessExceptionHandlingTest extends SingleBrowserTest {
 
     @Test
     public void testExceptionHandlingOnUIAccess() throws Exception {
@@ -15,17 +15,21 @@ public class UIAccessExceptionHandlingTest extends MultiBrowserTest {
         $(ButtonElement.class).first().click();
         assertLogTexts(
                 "1. Exception caught on get: java.util.concurrent.ExecutionException",
-                "0. Exception caught on execution with ConnectorErrorEvent : java.util.concurrent.ExecutionException");
+                "0. Exception caught on execution with ConnectorErrorEvent : java.lang.RuntimeException");
 
         $(ButtonElement.class).get(1).click();
         assertLogTexts(
                 "1. Exception caught on get: java.util.concurrent.ExecutionException",
-                "0. Exception caught on execution with ErrorEvent : java.util.concurrent.ExecutionException");
+                "0. Exception caught on execution with ErrorEvent : java.lang.RuntimeException");
 
         $(ButtonElement.class).get(2).click();
         assertLogTexts(
                 "1. Exception caught on get: java.util.concurrent.ExecutionException",
-                "0. Exception caught on execution with ConnectorErrorEvent : java.util.concurrent.ExecutionException");
+                "0. Exception caught on execution with ConnectorErrorEvent : java.lang.RuntimeException");
+
+        $(ButtonElement.class).get(3).click();
+        assertLogText(0,
+                "0. Exception caught on execution with ConnectorErrorEvent : java.lang.NullPointerException");
     }
 
     private void assertLogTexts(String first, String second) {