[🐛 Bug]: org.openqa.selenium.WebDriverException: Unable to route (POST) /session/0405a24bf1d06c6ae8402b8d12027934/...

[joerg1985]

What happened?

When executing a command on a just timedout session a WebDriverException: Unable to route ... is raised.

How can we reproduce the issue?

For some seconds after the session timedout there is a inconsistent state, until the sessionCleanupNodeService of the LocalNode did run. The LocalSessionMap still tells the session does exist, but the Node.isSessionOwner does return false. This will prevent the route inside the Node is matched and lead to the WebDriverException: Unable to route ....

Relevant log output

N/A

Operating System

Win 10 x64

Selenium version

4.17.1

What are the browser(s) and version(s) where you see this issue?

N/A

What are the browser driver(s) and version(s) where you see this issue?

N/A

Are you using Selenium Grid?

N/A

[github-actions]

@joerg1985, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[diemol]

How do you think Grid should behave in this case? Should the exception be more clear?

[joerg1985]

@diemol At this time the exception is raised the session is allready timed out.
So i would expect a NoSuchSessionException and not something that indicates an grid bug.

[diemol]

Do you know where this exception was shown? I can tweak the response we send to the user.

[joerg1985]

It is raised here:

selenium/java/src/org/openqa/selenium/grid/web/RoutableHttpClientFactory.java

Lines 60 to 68 in 2b59996

	HttpResponse response = new HttpResponse();
	response.setStatus(404);
	response.setContent(
	asJson(
	ImmutableMap.of(
	"value", request.getUri(),
	"message", "Unable to route " + request,
	"error", UnsupportedCommandException.class.getName())));
	return response;

Here the session map still returns the session, but the following ownership check fails:

selenium/java/src/org/openqa/selenium/grid/node/Node.java

Lines 149 to 151 in 2b59996

	getSessionId(req.getUri())
	.map(SessionId::new)
	.map(this::isSessionOwner)

[github-actions]

This issue is looking for contributors.

Please comment below or reach out to us through our IRC/Slack/Matrix channels if you are interested.

[VietND96]

@joerg1985, do you have a test on which command can return that error when session just timed out?
Since I tried to reproduce but could not see that error, only saw selenium.common.exceptions.InvalidSessionIdException: Message: Cannot find session with id: 05592d304812288f6c153de8529b0b28

[joerg1985]

@VietND96 i could reproduce the issue with the code below.

One solution might be to extend the org.openqa.selenium.grid.data.Session with the node id.
This could be used to check the ownership of the session instead of calling Node.isSessionOwner.
And might be helpful in other areas too, e.g. processing a slightly delayed NodeRestartedEvent, this could be the root cause of #14322
But i have not tried so far and there might be side effects.

public static void main(String[] args) throws Exception {
        org.openqa.selenium.grid.Bootstrap.main(("hub --host 127.0.0.1 --port 4444").split(" "));
        org.openqa.selenium.grid.Bootstrap.main(("node --host 127.0.0.1 --port 5555 --session-timeout 20 --selenium-manager true").split(" "));

        try {

            ClientConfig config = ClientConfig.defaultConfig()
                    .baseUri(URI.create("http://localhost:4444"));

            var options = new ChromeOptions();

            options.addArguments("--disable-search-engine-choice-screen");

            for (int i = 0; i < 10; i++) {

                WebDriver driver = new RemoteWebDriver(URI.create("http://localhost:4444").toURL(), options);//RemoteWebDriver.builder().config(config).oneOf(options).build();

                // ensure the session reached it's timeout
                Thread.sleep(20000);

                try {
                    driver.getTitle();
                } catch (NoSuchSessionException e) {
                    // the correct exception, the internal cleanup of LocalNode.currentSessions has been
                    // run between the session timeout and the getTitle command.
                    continue;
                } catch (WebDriverException e) {
                    /**
                     * this is the not expected state
                     */
                    throw e;
                }

            }
        } catch (Exception e) {
            e.printStackTrace();
        } finally {
            // kill the server
            System.exit(0);
        }

    }

[VietND96]

Adding NodeId to org.openqa.selenium.grid.data.Session would be a huge update, and it also adds a reverse dependency between Session <-> Node

[joerg1985]

You are right, the Node is currently not aware of the other Grid components.
Another option might be to replace the Guava Cache, as it makes entries unreachable before they are handled by the cleanup.

@diemol is it a long term goal to remove Guava from the server, like from the client?

[diemol]

I believe we did not think about the server, but I would not be opposed to that.

What would be the alternative?

[VietND96]

After walkthrough implementation, I see the event SessionClosedEvent is sent and listened by components properly.
I also added a fix by calling getSession(), which should return nonNull before going to check isSessionOwner(). Then we can get the right exception NoSuchSessionException. I added a unit test to verify it in PR #14628

[joerg1985]

@diemol One option would be to allow the node to answer with a "NoSuchSessionException" as proposed by @VietND96 as soon as a sessionId is set and the session is unkown or not owned by the node. Currently the node does answer with WebDriverException: Unable to route in this case.

To implement this i would move the .map(this::isSessionOwner) to the ForwardWebDriverCommand and drop the NodeTest.shouldOnlyRespondToWebDriverCommandsForSessionsTheNodeOwns test as it is useless now.

[VietND96]

I tried to update it as you suggested, move it to ForwardWebDriverCommand, and update the unit test without random UUID. Please check if it makes sense to you.

[github-actions]

This issue has been automatically locked since there has not been any recent activity since it was closed. Please open a new issue for related bugs.