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

…12531) (CP: 9.0) (#12547) Fixes #12100
vaadin · Dec 9, 2021 · 8067d7e · 8067d7e
1 parent 1ecc2c8
commit 8067d7e
Show file tree

Hide file tree

Showing 2 changed files with 62 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;
 
@@ -527,9 +528,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);
@@ -708,7 +724,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
@@ -16,6 +16,9 @@
 
 package com.vaadin.flow.component;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -24,13 +27,18 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.commons.io.IOUtils;
 import org.hamcrest.CoreMatchers;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.ArgumentMatchers;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.impl.SimpleLogger;
 
 import com.vaadin.flow.component.internal.PendingJavaScriptInvocation;
 import com.vaadin.flow.component.page.History;
@@ -79,13 +87,16 @@
 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.flow.shared.Registration;
 import com.vaadin.tests.util.AlwaysLockedVaadinSession;
 import com.vaadin.tests.util.MockUI;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mockStatic;
 
 public class UITest {
 
@@ -181,11 +192,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;
@@ -458,7 +475,7 @@ public void unsetSession_detachEventIsFiredForUIChildren()
     }
 
     @Test
-    public void unserSession_datachEventIsFiredForElements() {
+    public void unsetSession_detachEventIsFiredForElements() {
         UI ui = createTestUI();
 
         List<ElementDetachEvent> events = new ArrayList<>();
@@ -480,6 +497,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();