[🚀 Feature]: Allow specifying arguments to open for firefox.Service log

[vringar]

Feature and motivation

The current log_file argument only lets users supply the name of the log file but assumes that all users want it opened with a+ mode and utf-8 encoding. While this works for most users, it prevents certain use cases, as explained below. The PR #10704 makes this more flexible while maintaining full backwards compatibility.

Motivation:
As part of OpenWPM we use Selenium to drive multiple Firefox browsers in parallel.

To capture, enrich and redirect the browser's logging output, we use named pipes that look like normal log files to Selenium.
However pipes don't support seeking so the need to be opened with w and not with a+ as the firefox.Service currently does. We have previously monkeypatched this but that approach has led to us breaking every time a new Selenium release comes out. A newer approach that tries to call firefox.Service.__init__ and open the log_file afterwards doesn't work at all, so I wanted to check if there was an appetite to address this issue upstream.

Usage example

from selenium.webdriver import Firefox
from selenium.webdriver.firefox.service import Service
import os

os.mkfifo("my_custom_pipe", 0x600)
service = Service(log_path={"file":"my_custom_pipe", "mode":"w"})
driver = Firefox(service=service)
# Driver now logs to pipe and can be read from process
with open("my_custom_pipe", "rt") as f:
    for line in f:
        self.logger.debug("BROWSER %i: driver: %s" % (123, line.strip()))

[github-actions]

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

[vringar]

Full transparency: I'm filing this as a feature request, because the original bug/implementation stalled and maybe with this more accurate framing it is more likely to get approved.

[symonk]

My apologies, had a lot going on recently. Let's make sure we add an extensible way to do this if we are adding it. I am in the process of tidying up, documenting and type hinting the service code. Q: as it stands can you just pass the file object directly as log_path after opening it on in your own code? (or the file descriptor int itself after opening should also suffice).

Edit: I just looked at firefox specifically and it's not so simple based on how it's implemented, quite restrictive in what is being enforced there, makes sense to me to open that up and allow client code to utilise the underlying stdout/stderr for the subprocess however they see fit.

Just defining that consistent API is key here.

edit2: Even more confusing, some are shovelled as service args; other into the subprocess, need to dissect more...

Summary:

• firefox.service seems to hand off after (quite restrictively) opening the file path provided in a+ and pass into the subprocess stdout/stderr.

• chromium based stuff on the other hand, uses log_path to just append a flag to the subprocess through a --log-path= flag

so quite a bit of difference there.

---

I'm wondering if we just expose more control in general to the underlying subprocess.Popen does this solve the issue here? in that you could pass a file object with a valid file descriptor directly as log_file=... ? which is subsequently handed off to stdout=..., stderr=... ? the downside being it would require the client to correctly close the context (which the webdriver is doing atm I think as far as non chromium based stuff is concerned.

[vringar]

@symonk welcome back! No need to apologize. I'm sorry if I came off as rude/demanding.

I'm wondering if we just expose more control in general to the underlying subprocess.Popen does this solve the issue here? in that you could pass a file object with a valid file descriptor directly as log_file=... ? which is subsequently handed off to stdout=..., stderr=... ? the downside being it would require the client to correctly close the context (which the webdriver is doing atm I think as far as non chromium based stuff is concerned.

Is this something I should investigate/prototype or was this just thinking out loud on your part?

[diemol]

@titusfortner was this addressed with your recent changes?

[titusfortner]

No, that's the PR I'm still working on — #12103

Different browsers have different behavior, so looking to get it all the same. One log_output() method that accepts string for file name, opened file (TextIOWrapper), or STDOUT.

This should satisfy this issue, please let me know if you have feedback.

[vringar]

Closed by #12103

[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.