Repository navigation
[🐛 Bug]: When taking a screenshot fails, the resulting NameError is misinterpreted as a lack of driver support for screenshots #12799
Description
What happened?
Chromedriver sometimes returns {value: null} when you GET /session/<id>/screenshot instead of returning a string. I think that's probably a chromedriver bug.
In the selenium ruby bindings, in lib/selenium/webdriver/common/takes_screenshot.rb , any NameError is treated as "driver does not support screenshots".
When nil.unpack1('m') is called (because a null value was returned from chromedriver), this is reported as "driver does not support screenshots. This makes diagnosing the actual issue significantly more difficult!
How can we reproduce the issue?
Use chromedriver
Load a page with an iframe
Switch to the iframe
Take a screenshotRelevant log output
[53, 62] in /home/daniel/.gem/ruby/3.2.2/gems/selenium-webdriver-4.12.0/lib/selenium/webdriver/common/takes_screenshot.rb
53: when :base64
54: full_page ? full_screenshot : screenshot
55: when :png
56: as = screenshot_as(:base64, full_page: full_page)
57: debugger unless as
=> 58: as.unpack1('m')
59: else
60: raise Error::UnsupportedOperationError, "unsupported format: #{format.inspect}"
61: end
62: rescue NameError
(byebug) as
nil
(byebug) screenshot_as(:base64, full_page: full_page).length
253940Operating System
Ubuntu
Selenium version
4.12.0 ruby
What are the browser(s) and version(s) where you see this issue?
Chrome 115
What are the browser driver(s) and version(s) where you see this issue?
Chromedriver 115
Are you using Selenium Grid?
No
Activity
github-actions commentedon Sep 21, 2023
@DanielHeath, 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-templatelabel.If the issue is a question, add the
I-questionlabel.If the issue is valid but there is no time to troubleshoot it, consider adding the
help wantedlabel.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 answerlabel.Thank you!
DanielHeath commentedon Sep 21, 2023
Looking further into this - it appears that if you use
Timeout.timeout, theNet::HTTPclient used to communicate with chromedriver can get into a broken state.This is because
Timeout.timeoutasynchronously raises an exception in the calling thread, which interrupts operations which were not expected to be able to fail.In
ruby-3.2.2/lib/ruby/3.2.0/net/http.rbline 1855, the request is written to the socket; on line 1862 the response is read from the socket.However, if a timeout exception occurs before the response arrives, the response is not read from the socket. This causes the next request made with that
Net::HTTPclient to read the response from the interrupted request (since it's queued on the socket and nothing else has read it).titusfortner commentedon Sep 21, 2023
So what code reproduces this?
Getting null back from the screenshot endpoint needs to be raised as a bug to Google.
Http client timeouts should end the session completely.
DanielHeath commentedon Sep 22, 2023
So far, I can confirm (via
mitmproxy) thatchromedriversends the correct response, so no bug needs to be raised to google.By step-through debugging with
mitmproxyin the other window, I can confirm thatnet/httpreturns the wrong response.This is fixed if anything causes the socket to be closed and re-opened (eg if at least 2 seconds pass, as that's the keep-alive timeout in net/http).
Because
Timeoutuses::Kernel.catch(instance)andthrow(instance), the exception handlers innet/httpare skipped.I'm still working to get a minimal reproduction on that, but I expect to have it figured out soon.
DanielHeath commentedon Sep 22, 2023
This is fixed in timeout 0.4.0 (see also rails/rails#29333 , ruby/timeout#30 )
titusfortner commentedon Sep 22, 2023
So this is a bug in net/http? Presumably that is the library that is using the timeout method correctly.
diemol commentedon Nov 24, 2023
Is there something for us to do here or can this be closed?
titusfortner commentedon Nov 25, 2023
I went down a rabbit hole last time I looked at this trying to parse which libraries were supposed to be doing which things. What I recall is that net/http was the library that people agreed was properly handling the timeout issues. Can the user just require the latest version of timeout for this to work?
titusfortner commentedon Jan 20, 2024
closing as I don't have enough information to know what needs to be done to fix something in selenium code.
DanielHeath commentedon Jan 21, 2024
There are various ways
Timeoutcan break things unexpectedly.However I raised this issue specifically about the use of
NameErrorto check for screenshot support.Surely drivers have a way to report screenshot capability other than raising an exception when you try to use them?
Catching any
NameErrorand assuming it means "no screenshot support" made debugging the code much more difficult, and ruby has e.g.respond_to?(:screenshot)[rb] explicitly check for whether driver supports full page screensho…
titusfortner commentedon Jan 21, 2024
Oh, now I get it. I was being more clever than I should be. I changed to explicitly check for it.
github-actions commentedon Feb 21, 2024
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.