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]: DevTools timeout is hardcoded and not sufficient for some commands #14912

Closed
dennisoelkers opened this issue Dec 18, 2024 · 13 comments · Fixed by #14931 · May be fixed by #15059
Closed

[🐛 Bug]: DevTools timeout is hardcoded and not sufficient for some commands #14912

dennisoelkers opened this issue Dec 18, 2024 · 13 comments · Fixed by #14931 · May be fixed by #15059

Comments

@dennisoelkers
Copy link
Contributor

What happened?

(If this is a bug or a feature is certainly discussable. I decided to file it as a bug, because it is effectively blocking functionality in our code that should work without having to add a feature)

The timeout used for executing DevTools commands in the Java library is hardcoded to 10 seconds. For some commands (e.g. printing a large/complex page to PDF) or on overloaded systems, this might not be enough. It should be configurable.

How can we reproduce the issue?

-

Relevant log output

-

Operating System

Linux

Selenium version

Java 4.22.0

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

Chrome 124

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

ChromeDriver 124.0.6367.201

Are you using Selenium Grid?

No response

Copy link

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

@navin772
Copy link
Contributor

Hi @dennisoelkers, thanks for reporting this issue. I think this is a valid request, I will make the required changes and open a PR!

@navin772
Copy link
Contributor

@dennisoelkers can you provide the reproducible code you were having issues with so I can test the changes?

@navin772
Copy link
Contributor

navin772 commented Jan 9, 2025

Hi @dennisoelkers, as per #14931 (review), we are moving to BiDi completely and CDP functionality will be removed depreciated.

But, we can increase the hardcoded timeout value. Do you have an approx timeout value that will suffice your tests?

@diemol
Copy link
Member

diemol commented Jan 9, 2025

To add to the comment above, CDP will not be removed immediately; there will be an extended migration period.

@navin772
Copy link
Contributor

navin772 commented Jan 9, 2025

Yeah, I meant to say depreciated, thanks for correcting!

@dennisoelkers
Copy link
Contributor Author

dennisoelkers commented Jan 9, 2025

Hey @navin772,

But, we can increase the hardcoded timeout value. Do you have an approx timeout value that will suffice your tests?

We are currently using 180s - it is very conservative (as in too high), but has served us well so far.

In our case, we are using CDP to print a page to PDF. Is it possible to do this using BiDi already? How is BiDi handling the timeout for operations like this, which might take longer?

@navin772
Copy link
Contributor

navin772 commented Jan 9, 2025

@dennisoelkers we were thinking of increasing it to 30s max.

For BiDi, I think this is the documentation - https://www.selenium.dev/documentation/webdriver/bidi/w3c/browsing_context/#print-page. Implementation here.

@diemol can you confirm if the timeout is configurable in BiDi?

@diemol
Copy link
Member

diemol commented Jan 9, 2025

I don't think so. If we want to make a change, that is where we should spend time.

@pujagani, do you know?

@pujagani
Copy link
Contributor

I do not think the timeout is configurable in BiDi. Currently, It is set to 30 seconds. I think we should make it configurable so end users have that flexibility.

@dennisoelkers
Copy link
Contributor Author

@navin772, @diemol: Would it be so hard to add a configurable timeout to CDP? What we did locally to fix the issue we were seeing with the timeout being too low, was that we monkey-patched the DevTools class and added a sendWithTimeout(Command<V>, Duration) method.

We want to get rid of the monkey-patching part, but the interface is quite good for us, because we want to use it for certain feature (e.g. the print command), but for others the default timeout is okay. Would that be an option?

@diemol
Copy link
Member

diemol commented Jan 10, 2025

@dennisoelkers would you like to send a PR for that?

@dennisoelkers
Copy link
Contributor Author

@diemol: Sure, here it is: #15059

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants