fix: delay session expiration handling to prevent canceling ongoing navigation (#19983) (CP: 23.5)

flow-client/src/main/java/com/vaadin/client/communication/MessageHandler.java

@@ -431,9 +431,14 @@ assert getServerId(valueMap) == -1
                     if (nextResponseSessionExpiredHandler != null) {
                         nextResponseSessionExpiredHandler.execute();
                     } else if (uiState != UIState.TERMINATED) {
-                        registry.getSystemErrorHandler()
-                                .handleSessionExpiredError(null);
                         registry.getUILifecycle().setState(UIState.TERMINATED);
+                        // Delay the session expiration handling to prevent
+                        // canceling potential ongoing page redirect/reload
+                        Scheduler.get().scheduleFixedDelay(() -> {
+                            registry.getSystemErrorHandler()
+                                    .handleSessionExpiredError(null);
+                            return false;
+                        }, 250);
                     }
                 } else if (meta.containsKey("appError")
                         && uiState != UIState.TERMINATED) {

flow-client/src/test-gwt/java/com/vaadin/client/GwtMessageHandlerTest.java

@@ -152,12 +152,13 @@ public void handleSessionExpiredError(String details) {
 
         @Override
         public void handleUnrecoverableError(String caption, String message,
-                                             String details, String url, String querySelector) {
+                String details, String url, String querySelector) {
             unrecoverableErrorHandled = true;
         }
     }
 
-    private static class TestApplicationConfiguration extends ApplicationConfiguration {
+    private static class TestApplicationConfiguration
+            extends ApplicationConfiguration {
         @Override
         public String getApplicationId() {
             return "test-application-id";
@@ -223,8 +224,7 @@ public void testMessageProcessing_moduleDependencyIsHandledBeforeApplyingChanges
             assertEquals(ResourceLoader.class.getName(),
                     eventsOrder.sources.get(0));
             // the second one is applying changes to StatTree
-            assertEquals(StateTree.class.getName(),
-                    eventsOrder.sources.get(1));
+            assertEquals(StateTree.class.getName(), eventsOrder.sources.get(1));
         });
     }
 
@@ -301,11 +301,13 @@ public void testHandleJSON_uiTerminated_sessionExpiredMessageNotShown() {
 
         doAssert(() -> {
             // then: no session expire and unrecoverable error handling expected
-            assertFalse("Session Expired Message handling is not expected " +
-                        "when the page is being redirected",
+            assertFalse(
+                    "Session Expired Message handling is not expected "
+                            + "when the page is being redirected",
                     getSystemErrorHandler().sessionExpiredMessageHandled);
-            assertFalse("Unrecoverable Error Message handling was not " +
-                        "expected when the page is being redirected",
+            assertFalse(
+                    "Unrecoverable Error Message handling was not "
+                            + "expected when the page is being redirected",
                     getSystemErrorHandler().unrecoverableErrorHandled);
             assertEquals(UILifecycle.UIState.TERMINATED,
                     getUILifecycle().getState());
@@ -333,11 +335,13 @@ public void testHandleJSON_uiTerminated_unrecoverableErrorMessageNotShown() {
 
         doAssert(() -> {
             // then: no session expire and unrecoverable error handling expected
-            assertFalse("Session Expired Message handling is not expected " +
-                        "when the page is being redirected",
+            assertFalse(
+                    "Session Expired Message handling is not expected "
+                            + "when the page is being redirected",
                     getSystemErrorHandler().sessionExpiredMessageHandled);
-            assertFalse("Unrecoverable Error Message handling was not " +
-                        "expected when the page is being redirected",
+            assertFalse(
+                    "Unrecoverable Error Message handling was not "
+                            + "expected when the page is being redirected",
                     getSystemErrorHandler().unrecoverableErrorHandled);
             assertEquals(UILifecycle.UIState.TERMINATED,
                     getUILifecycle().getState());
@@ -369,7 +373,7 @@ public void testHandleJSON_sessionExpiredAndUIRunning_sessionExpiredMessageShown
                     getSystemErrorHandler().unrecoverableErrorHandled);
             assertEquals(UILifecycle.UIState.TERMINATED,
                     getUILifecycle().getState());
-        });
+        }, 300);
     }
 
     public void testHandleJSON_unrecoverableErrorAndUIRunning_unrecoverableErrorMessageShown() {
@@ -416,8 +420,7 @@ private void doAssert(Runnable assertions) {
         doAssert(assertions, 100);
     }
 
-    private void doAssert(Runnable assertions,
-                          int assertDelayInMillis) {
+    private void doAssert(Runnable assertions, int assertDelayInMillis) {
         delayTestFinish(500);
         new Timer() {
             @Override

flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java

@@ -287,6 +287,14 @@ public void handleRpc(UI ui, Reader reader, VaadinRequest request)
                 getLogger().info(
                         "Ignoring old duplicate message from the client. Expected: "
                                 + expectedId + ", got: " + requestId);
+            } else if (rpcRequest.isUnloadBeaconRequest()) {
+                getLogger().debug(
+                        "Ignoring unexpected message id from the client on UNLOAD request. "
+                                + "This could happen for example during login process, if concurrent requests "
+                                + "are sent to the server and one of those changes the session identifier, "
+                                + "causing an UIDL request to be rejected because of session expiration. "
+                                + "Expected sync id: {}, got {}.",
+                        expectedId, requestId);
             } else {
                 /*
                  * If the reason for ending up here is intermittent, then we

flow-tests/test-frontend/vite-embedded-webcomponent-resync/src/test/java/com/vaadin/viteapp/BasicComponentIT.java

@@ -21,10 +21,13 @@
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.openqa.selenium.By;
 import org.openqa.selenium.Keys;
 import org.openqa.selenium.StaleElementReferenceException;
+import org.openqa.selenium.support.ui.ExpectedConditions;
 
 import com.vaadin.flow.testutil.ChromeDeviceTest;
+import com.vaadin.testbench.TestBenchElement;
 
 public class BasicComponentIT extends ChromeDeviceTest {
 
@@ -50,18 +53,20 @@ public void session_resynced_webcomponent_is_active() throws Exception {
         Assert.assertEquals("Authentication failure",
                 getAuthenticationResult());
 
+        TestBenchElement input = $("login-form").first().$("input").first();
+
         // simulate expired session by invalidating current session
         session.invalidate();
 
         // init request to resynchronize expired session and recreate components
         clickButton();
 
-        try {
-            // it seems WebDriver needs also sync to new session
-            setUsername("");
-        } catch (StaleElementReferenceException ex) {
-            // NOP
-        }
+        // Wait for web component to be detached, session expiration message
+        // should be delivered by PUSH long polling connection
+        waitUntil(ExpectedConditions.stalenessOf(input));
+
+        waitForElementPresent(By.tagName("login-form"));
+        waitUntil(d -> "".equals(getAuthenticationResult()));
 
         // check if web component works again
         setUsername("admin");

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/InternalErrorIT.java

@@ -83,6 +83,8 @@ public void enableSessionExpiredNotification_sessionExpired_notificationShown()
         // Just click on any button to make a request after killing the session
         clickButton(CLOSE_SESSION);
 
+        waitUntil(d -> isSessionExpiredNotificationPresent());
+
         Assert.assertTrue("After enabling the 'Session Expired' notification, "
                 + "the page should not be refreshed "
                 + "after killing the session", isMessageUpdated());

flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/RouterSessionExpirationIT.java

@@ -34,11 +34,13 @@ public void should_HaveANewSessionId_when_NavigationAfterSessionExpired() {
         navigateToSesssionExpireView();
         // expired session causes page reload, after the page reload there will
         // be a new session
-        Assert.assertNotEquals(sessionId, getSessionId());
-        sessionId = getSessionId();
+        // Assert.assertNotEquals(sessionId, getSessionId());
+        waitUntil(d -> !sessionId.equals(getSessionId()));
+
+        String newSessionId = getSessionId();
         navigateToAnotherView();
         // session is preserved
-        Assert.assertEquals(sessionId, getSessionId());
+        Assert.assertEquals(newSessionId, getSessionId());
     }
 
     @Test