fix: delay session expiration handling to prevent canceling ongoing n…

…avigation (#19983) (#20448) Attempts to fix the synchronization issue related to the usage of the Login reported in #12640. The Login component sends the UIDL request for the login event to the server and concurrently submits the form. If processing the form submission performs a session ID change and a request redirect, the UIDL requests might fail with a session expiration response. The Flow client then can cancel the first redirect because it reloads the page due to the session expiration. Lastly, the beacon request hits again a valid session, but a resynchronization is triggered because the previous UIDL request was rejected. This change delays a bit the session expiration handling on Flow client, to allow a potential redirect to complete without being cancelled. However, the client application is immediately set in TERMINATED state. Co-authored-by: Marco Collovati <[email protected]>
vaadin · Nov 12, 2024 · f8ac87d · f8ac87d
1 parent 7fadbd2
commit f8ac87d
Show file tree

Hide file tree

Showing 9 changed files with 79 additions and 57 deletions.
diff --git a/flow-client/src/main/java/com/vaadin/client/communication/MessageHandler.java b/flow-client/src/main/java/com/vaadin/client/communication/MessageHandler.java
@@ -456,9 +456,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) {

diff --git a/flow-client/src/test-gwt/java/com/vaadin/client/GwtMessageHandlerTest.java b/flow-client/src/test-gwt/java/com/vaadin/client/GwtMessageHandlerTest.java
@@ -159,12 +159,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";
@@ -230,8 +231,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));
         });
     }
 
@@ -308,11 +308,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());
@@ -340,11 +342,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());
@@ -376,7 +380,7 @@ public void testHandleJSON_sessionExpiredAndUIRunning_sessionExpiredMessageShown
                     getSystemErrorHandler().unrecoverableErrorHandled);
             assertEquals(UILifecycle.UIState.TERMINATED,
                     getUILifecycle().getState());
-        });
+        }, 300);
     }
 
     public void testHandleJSON_unrecoverableErrorAndUIRunning_unrecoverableErrorMessageShown() {
@@ -423,8 +427,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

diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/ServerRpcHandler.java
@@ -298,6 +298,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

diff --git a/...ed-webcomponent-resync-longpolling/src/test/java/com/vaadin/viteapp/BasicComponentIT.java b/...ed-webcomponent-resync-longpolling/src/test/java/com/vaadin/viteapp/BasicComponentIT.java
@@ -28,11 +28,14 @@
 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.server.InitParameters;
 import com.vaadin.flow.testutil.ChromeDeviceTest;
+import com.vaadin.testbench.TestBenchElement;
 
 public class BasicComponentIT extends ChromeDeviceTest {
 
@@ -63,19 +66,16 @@ 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();
-        waitForWebComponent("login-form");
-
-        // init request to resynchronize expired session and recreate components
-        clickButton();
+        // Wait for web component to be detached, session expiration message
+        // should be delivered by PUSH long polling connection
+        waitUntil(ExpectedConditions.stalenessOf(input));
 
-        try {
-            // it seems WebDriver needs also sync to new session
-            setUsername("");
-        } catch (StaleElementReferenceException ex) {
-            // NOP
-        }
+        waitForElementPresent(By.tagName("login-form"));
+        waitUntil(d -> "".equals(getAuthenticationResult()));
 
         // check if web component works again
         setUsername("admin");

diff --git a/...te-embedded-webcomponent-resync-ws/src/test/java/com/vaadin/viteapp/BasicComponentIT.java b/...te-embedded-webcomponent-resync-ws/src/test/java/com/vaadin/viteapp/BasicComponentIT.java
@@ -28,11 +28,14 @@
 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.server.InitParameters;
 import com.vaadin.flow.testutil.ChromeDeviceTest;
+import com.vaadin.testbench.TestBenchElement;
 
 public class BasicComponentIT extends ChromeDeviceTest {
 
@@ -63,19 +66,15 @@ 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();
-        waitForWebComponent("login-form");
-
-        // init request to resynchronize expired session and recreate components
-        clickButton();
+        // Wait for web component to be detached, session expiration message
+        // should be delivered over websocket
+        waitUntil(ExpectedConditions.stalenessOf(input));
 
-        try {
-            // it seems WebDriver needs also sync to new session
-            setUsername("");
-        } catch (StaleElementReferenceException ex) {
-            // NOP
-        }
+        waitForElementPresent(By.tagName("login-form"));
+        waitUntil(d -> "".equals(getAuthenticationResult()));
 
         // check if web component works again
         setUsername("admin");

diff --git a/...embedded-webcomponent-resync-wsxhr/src/test/java/com/vaadin/viteapp/BasicComponentIT.java b/...embedded-webcomponent-resync-wsxhr/src/test/java/com/vaadin/viteapp/BasicComponentIT.java
@@ -28,11 +28,14 @@
 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.server.InitParameters;
 import com.vaadin.flow.testutil.ChromeDeviceTest;
+import com.vaadin.testbench.TestBenchElement;
 
 public class BasicComponentIT extends ChromeDeviceTest {
 
@@ -63,19 +66,15 @@ 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();
-        waitForWebComponent("login-form");
-
-        // init request to resynchronize expired session and recreate components
-        clickButton();
+        // Wait for web component to be detached, session expiration message
+        // should be delivered over websocket
+        waitUntil(ExpectedConditions.stalenessOf(input));
 
-        try {
-            // it seems WebDriver needs also sync to new session
-            setUsername("");
-        } catch (StaleElementReferenceException ex) {
-            // NOP
-        }
+        waitForElementPresent(By.tagName("login-form"));
+        waitUntil(d -> "".equals(getAuthenticationResult()));
 
         // check if web component works again
         setUsername("admin");

diff --git a/.../vite-embedded-webcomponent-resync/src/test/java/com/vaadin/viteapp/BasicComponentIT.java b/.../vite-embedded-webcomponent-resync/src/test/java/com/vaadin/viteapp/BasicComponentIT.java
@@ -28,11 +28,14 @@
 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.server.InitParameters;
 import com.vaadin.flow.testutil.ChromeDeviceTest;
+import com.vaadin.testbench.TestBenchElement;
 
 public class BasicComponentIT extends ChromeDeviceTest {
 
@@ -63,19 +66,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();
-        waitForWebComponent("login-form");
 
         // 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");

diff --git a/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/InternalErrorIT.java b/flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/InternalErrorIT.java
@@ -90,6 +90,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());

diff --git a/.../test-root-context/src/test/java/com/vaadin/flow/uitest/ui/RouterSessionExpirationIT.java b/.../test-root-context/src/test/java/com/vaadin/flow/uitest/ui/RouterSessionExpirationIT.java
@@ -41,11 +41,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