fix: Do not throw NPE for UI.access called when session has expired (#…

…12531) (#12591) Fixes #12100 Co-authored-by: Artur <[email protected]>
vaadin · Dec 16, 2021 · ffe244e · ffe244e
1 parent 87c1d28
commit ffe244e
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/component/UI.java b/flow-server/src/main/java/com/vaadin/flow/component/UI.java
@@ -23,6 +23,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.regex.Pattern;
 
@@ -515,9 +516,24 @@ public void handleError(Exception exception) {
                     if (command instanceof ErrorHandlingCommand) {
                         ErrorHandlingCommand errorHandlingCommand = (ErrorHandlingCommand) command;
                         errorHandlingCommand.handleError(exception);
-                    } else {
+                    } else if (getSession() != null) {
                         getSession().getErrorHandler()
                                 .error(new ErrorEvent(exception));
+                    } else {
+                        /*
+                         * The session has expired after `ui.access` was called.
+                         * It makes no sense to pollute the logs with a
+                         * UIDetachedException at this point.
+                         */
+                        if (exception instanceof ExecutionException
+                                && ((ExecutionException) exception)
+                                        .getCause() instanceof UIDetachedException) {
+                            getLogger().debug(exception.getMessage(),
+                                    exception);
+                        } else {
+                            getLogger().error(exception.getMessage(),
+                                    exception);
+                        }
                     }
                 } catch (Exception e) {
                     getLogger().error(e.getMessage(), e);
@@ -696,7 +712,7 @@ public ReconnectDialogConfiguration getReconnectDialogConfiguration() {
         return getNode().getFeature(ReconnectDialogConfigurationMap.class);
     }
 
-    private static Logger getLogger() {
+    Logger getLogger() {
         return LoggerFactory.getLogger(UI.class.getName());
     }
 

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/UITest.java b/flow-server/src/test/java/com/vaadin/flow/component/UITest.java
@@ -1,5 +1,6 @@
 package com.vaadin.flow.component;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -15,6 +16,7 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Matchers;
 import org.mockito.Mockito;
+import org.slf4j.Logger;
 
 import com.vaadin.flow.component.internal.PendingJavaScriptInvocation;
 import com.vaadin.flow.component.page.History;
@@ -61,6 +63,7 @@
 import com.vaadin.flow.server.VaadinResponse;
 import com.vaadin.flow.server.VaadinService;
 import com.vaadin.flow.server.VaadinServletRequest;
+import com.vaadin.flow.server.frontend.MockLogger;
 import com.vaadin.tests.util.AlwaysLockedVaadinSession;
 import com.vaadin.tests.util.MockUI;
 
@@ -163,11 +166,17 @@ public void elementIsBody() {
     }
 
     private static UI createTestUI() {
+        MockLogger mockLogger = new MockLogger();
         UI ui = new UI() {
             @Override
             public void doInit(VaadinRequest request, int uiId) {
 
             }
+
+            @Override
+            Logger getLogger() {
+                return mockLogger;
+            }
         };
 
         return ui;
@@ -439,7 +448,7 @@ public void unsetSession_detachEventIsFiredForUIChildren()
     }
 
     @Test
-    public void unserSession_datachEventIsFiredForElements() {
+    public void unsetSession_detachEventIsFiredForElements() {
         UI ui = createTestUI();
 
         List<ElementDetachEvent> events = new ArrayList<>();
@@ -461,6 +470,32 @@ public void unserSession_datachEventIsFiredForElements() {
         assertEquals(ui.getElement(), events.get(1).getSource());
     }
 
+    @Test
+    public void unsetSession_accessErrorHandlerStillWorks() throws IOException {
+        UI ui = createTestUI();
+        initUI(ui, "", null);
+
+        ui.getSession().access(() -> ui.getInternals().setSession(null));
+        ui.access(() -> {
+            Assert.fail("We should never get here because the UI is detached");
+        });
+
+        // Unlock to run pending access tasks
+        ui.getSession().unlock();
+
+        String logOutput = ((MockLogger) ui.getLogger()).getLogs();
+        String logOutputNoDebug = logOutput.replaceAll("^\\[Debug\\].*", "");
+
+        Assert.assertFalse(
+                "No NullPointerException should be logged but got: "
+                        + logOutput,
+                logOutput.contains("NullPointerException"));
+        Assert.assertFalse(
+                "No UIDetachedException should be logged but got: "
+                        + logOutputNoDebug,
+                logOutputNoDebug.contains("UIDetachedException"));
+    }
+
     @Test
     public void beforeClientResponse_regularOrder() {
         UI ui = createTestUI();