Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: delay session expiration handling to prevent canceling ongoing navigation #19983

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -376,7 +380,7 @@ public void testHandleJSON_sessionExpiredAndUIRunning_sessionExpiredMessageShown
getSystemErrorHandler().unrecoverableErrorHandled);
assertEquals(UILifecycle.UIState.TERMINATED,
getUILifecycle().getState());
});
}, 300);
}

public void testHandleJSON_unrecoverableErrorAndUIRunning_unrecoverableErrorMessageShown() {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,18 @@ public void session_resynced_webcomponent_is_active() throws Exception {

// simulate expired session by invalidating current session
session.invalidate();
waitForWebComponent("login-form");

// init request to resynchronize expired session and recreate components
clickButton();

waitForWebComponent("login-form");
try {
// it seems WebDriver needs also sync to new session
setUsername("");
} catch (StaleElementReferenceException ex) {
// NOP
}
waitUntil(d -> "".equals(getAuthenticationResult()));

// check if web component works again
setUsername("admin");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading