[🐛 Bug]: A selenium.common.exceptions.InvalidSessionIdException exception is raised when the print options are re-used from a session to another.

[tvataire]

What happened?

Current behavior :
Python language bindings for Selenium WebDriver raise a selenium.common.exceptions.InvalidSessionIdException exception when a PrintOptions instance is re-used by the print_page() method from a session to another.

Expected behavior :
No exception should be raised.

How can we reproduce the issue?

#!/bin/env python3
# -*- coding: utf8 -*-

import geckodriver_autoinstaller
from selenium.webdriver import Firefox
from selenium.webdriver.common.print_page_options import PrintOptions
from selenium.webdriver.firefox.options import Options

if __name__ == '__main__':
    geckodriver_autoinstaller.install()
    po = PrintOptions()
    po.margin_top = 0
    po.margin_left = 0
    po.margin_right = 0
    po.margin_bottom = 0
    do = Options()
    do.headless = True
    for idx in range(0, 2):
        driver = Firefox(options=do)
        try:
            print(driver.session_id)
            driver.print_page(po)
        finally:
            driver.close()

Relevant log output

/mnt/PycharmProjects/selenium/venv/bin/python /mnt/PycharmProjects/selenium/print_issue.py 
ac74ec4c-8e63-47e9-a679-9b8c7153e0ff
228366ed-1da3-4315-8808-49efe081f160
Traceback (most recent call last):
  File "/mnt/PycharmProjects/selenium/print_issue.py", line 22, in <module>
    driver.print_page(po)
  File "/mnt/PycharmProjects/selenium/venv/lib/python3.9/site-packages/selenium/webdriver/remote/webdriver.py", line 619, in print_page
    return self.execute(Command.PRINT_PAGE, options)['value']
  File "/mnt/PycharmProjects/selenium/venv/lib/python3.9/site-packages/selenium/webdriver/remote/webdriver.py", line 434, in execute
    self.error_handler.check_response(response)
  File "/mnt/PycharmProjects/selenium/venv/lib/python3.9/site-packages/selenium/webdriver/remote/errorhandler.py", line 247, in check_response
    raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.InvalidSessionIdException: Message: Got unexpected session id ac74ec4c-8e63-47e9-a679-9b8c7153e0ff


Process finished with exit code 1

Operating System

Debian Bullseye

Selenium version

Python 4.5.0

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

Firefox 102.3.0esr (64 bits)

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

GeckoDriver 0.31.0

Are you using Selenium Grid?

No response

[github-actions]

@tvataire, 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!

[titusfortner]

Yeah, looks like we're mutating the incoming payload
https://github.com/SeleniumHQ/selenium/blob/trunk/py/selenium/webdriver/remote/webdriver.py#L437-L438

For comparison, Ruby is making a copy first
https://github.com/SeleniumHQ/selenium/blob/trunk/rb/lib/selenium/webdriver/remote/bridge.rb#L614

I think the right answer is to put the copy() in the execute() method so it applies to all incoming objects. @symonk does that sound right? @tvataire would you like to make a PR with the fix?

[tvataire]

Hi,

@tvataire would you like to make a PR with the fix?

It's in progress :)

think the right answer is to put the copy() in the execute() method so it applies to all incoming objects. @symonk does that sound right?

Not sure.
Shouldn't the PrintOptions class restrict access to it's internal data to comply with the encapsulation principle of object-oriented programming ? This will prevent the problem from reoccurring elsewhere.

[tvataire]

Related PR is #11096

[symonk]

I need to look closer, but as @titusfortner mentions I think we probably don't want to be mutating the params map internally there.

edit: Thanks for the PR I will take a look as soon as possible

[titusfortner]

yeah, I'm not a fan of objects storing internal state in a dictionary and then providing access to that dictionary in a public method. We shouldn't mutate options in the execute() method, but @tvataire is correct that the class itself shouldn't allow this. Using copy is one way, but we might want to consider a different strategy for converting objects to dictionaries since Selenium uses this pattern in multiple places. https://stackoverflow.com/a/35282286/4072371

[tvataire]

we might want to consider a different strategy for converting objects to dictionaries since Selenium uses this pattern in multiple places.

Yes, what do you think about the "Option 4" and implements collections.abc.Mapping ?
It seems to be the more pythonic way to do that.

[tvataire]

An other fix for this bug, equivalent than the one suggested by @titusfortner, can be considered.
The Webdriver._wrap_value() method alter params of the Webdriver.execute() method and returns them in a new dictionary.
The SessionId is added to params before the Webdriver._wrap_value() method was called whereas this method doesn't affect its value.
The SessionId could therefore be added after the Webdriver._wrap_value() method is called without incidence on its result.
This fix the bug without mutating params in the PrintOptions class.
What do you think about this option ?

Related PR is #11121

[symonk]

I believe this was resolved in #11121

[github-actions]

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