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

[🐛 Bug]: PySelenium doesn't support a named pipe as a log file #10703

Closed
vringar opened this issue May 27, 2022 · 14 comments
Closed

[🐛 Bug]: PySelenium doesn't support a named pipe as a log file #10703

vringar opened this issue May 27, 2022 · 14 comments

Comments

@vringar
Copy link

vringar commented May 27, 2022

What happened?

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.

How can we reproduce the issue?

import os
from selenium.webdriver import Firefox
from selenium.webdriver.firefox.service import Service
pipe_name = "selenium_pipe.log"
os.mkfifo(pipe_name, 0o600)
service=Service(executable_path="firefox",log_path=pipe_name)
driver = Firefox(
    service=service
)
driver.get("https://example.com")

Relevant log output

File "/home/vringar/selenium_mvp/demo.py", line 9, in <module>
    service=Service(executable_path="firefox",log_path=pipe_name)
File "/home/stefan/.conda/envs/openwpm/lib/python3.10/site-packages/selenium/webdriver/firefox/service.py", line 50, in __init__
    log_file = open(log_path, "a+") if log_path else None
io.UnsupportedOperation: File or stream is not seekable.

Operating System

Linux

Selenium version

Python 4.1.0

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

Firefox 100

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

GeckoDriver 0.30.0

Are you using Selenium Grid?

No response

@github-actions
Copy link

@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 added a commit to vringar/selenium that referenced this issue May 27, 2022
By checking the error returned when trying to open the file
with `a+` and only using `w` when it is a pipe, there is no change in
behaviour wrt the common path

Fixes SeleniumHQ#10703
vringar added a commit to vringar/selenium that referenced this issue May 27, 2022
By checking the error returned when trying to open the file
with `a+` and only using `w` when it is a pipe, there is no change in
behaviour wrt the common path

Fixes SeleniumHQ#10703
@titusfortner
Copy link
Member

Looks like you already have the code; please PR your proposed changes and discuss whatever impact it might have on existing users. We have a *lot of work planned in Python for 4.3.

@titusfortner
Copy link
Member

Also, conceptually, is this a bug, or effectively a feature request to support another type of output?

@vringar
Copy link
Author

vringar commented May 27, 2022

Also, conceptually, is this a bug, or effectively a feature request to support another type of output?

Hmm, I was unsure about that as well. I just picked the bug report because it allowed me to show that the desired behavior is currently not supported.

@vringar
Copy link
Author

vringar commented May 27, 2022

Looks like you already have the code; please PR your proposed changes

Done

and discuss whatever impact it might have on existing users. We have a *lot of work planned in Python for 4.3.

I have done that in the PR. Should I do it here as well?

@symonk symonk added the C-py label May 27, 2022
@symonk
Copy link
Member

symonk commented May 27, 2022

Thanks @vringar seems ok to me, we could expose the mode to the user too to simplify the solution but your proposal seems ok to me too

vringar added a commit to vringar/selenium that referenced this issue May 31, 2022
By checking the error returned when trying to open the file
with `a+` and only using `w` when it is a pipe, there is no change in
behaviour wrt the common path

Fixes SeleniumHQ#10703
vringar added a commit to vringar/selenium that referenced this issue May 31, 2022
By checking the error returned when trying to open the file
with `a+` and only using `w` when it is a pipe, there is no change in
behaviour wrt the common path

Fixes SeleniumHQ#10703
vringar added a commit to vringar/selenium that referenced this issue May 31, 2022
This change allows users to specify how they want
to open their log file with a fallback to `a+`
if nothing else is specified

Fixes SeleniumHQ#10703
vringar added a commit to vringar/selenium that referenced this issue Jun 17, 2022
This change allows users to specify how they want
to open their log file with a fallback to `a+`
if nothing else is specified

Fixes SeleniumHQ#10703
vringar added a commit to vringar/selenium that referenced this issue Sep 22, 2022
This change allows users to specify how they want
to open their log file with a fallback to `a+`
if nothing else is specified

Fixes SeleniumHQ#10703
vringar added a commit to vringar/selenium that referenced this issue Sep 29, 2022
This change allows users to specify how they want
to open their log file with a fallback to `a+`
if nothing else is specified

Fixes SeleniumHQ#10703
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the I-stale Applied to issues that become stale, and eventually closed. label Mar 3, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 14 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2023
@titusfortner
Copy link
Member

I think I have a solution for this one that I'm working on.

@titusfortner titusfortner reopened this May 26, 2023
@github-actions github-actions bot removed the I-stale Applied to issues that become stale, and eventually closed. label May 26, 2023
@titusfortner
Copy link
Member

This PR ^^ will let you create the file ahead of time and pass it in rather than having to mess with tuples.
sorry it took us so long for me to dig into this code.

@vringar
Copy link
Author

vringar commented May 30, 2023

Looks great! Is there any way I can contribute to #12103 or #12030? Or should I just wait?

@titusfortner
Copy link
Member

I'm hoping to merge #12030 today
Whether #12103 is in 4.10 or 4.11 depends on when we release 4.10 (end of the week?) and when I can get someone else to review it. 😁

I just added docs for all the languages to show which languages support what logging options — https://www.selenium.dev/documentation/webdriver/drivers/service/#console-output

So we can prioritize fixing things

@diemol
Copy link
Member

diemol commented Jun 15, 2023

Implemented via #12103

@diemol diemol closed this as completed Jun 15, 2023
Copy link

github-actions bot commented Dec 9, 2023

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.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants