-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[dotnet] Added function for sending multiple DevTools command #13539
Conversation
This is something strange, and based what I see in changing API, I am strongly opiniated this should not be a part of selenium code base. Selenium provides a method to send single cdp command, I don't see problems how user might send multiple commands, even in parallel, up to user. But selenium should be aware of thread-safety. To be on the same page let's start from a simple problem: reproducible real scenario where selenium behaves incorrectly. And then we will see where the issue is and how we can resolve it. |
Can we fill out an issue to go with this PR with all the details? Thanks! |
I've rewritten the example a bit to demonstrate the issue I'm facing. Added the async modifier to the event handler and now use the singular SendCommand functions. Note that we use the SessionId argument from the event itself, therefore can't use the domains. Running this code will result in the first devToolsSession.SendCommand to hang until the timeout (30 secs) expired. Might be the thread is locked because it's not an async event handler to begin with?
Fine by me if we make a separate issue from that, and close this PR. |
@EdwinVanVliet can it be even more simplified? It would be really helpful to see the entire code, just to copy/paste and see a problem. PS:
Seems |
I think the problem is related to invoking a command while handling the event. I also tried converting the TargetAttached event to async, but without succes. Keeps hanging on sending the first command to DevTools.
|
I ran your example locally - exited with success:
Really, what if we start the discussion from the issue you face? I appreciate your contribution, it is making this project great. |
@EdwinVanVliet I spent couple of hours and now I see the issue is not related to that cdp command processor cannot handle sequential commands. I will dive deeper (don't promise). My first thoughts are we should clearly understand what commands we want to send! I mean what are correct commands we should send.. My next steps are to debug cdp conversation from browser perspective. |
I think it's also a question of what the end user wants to enable. For my own use case we enable Page, Runtime and Network. However someone else might have different needs. |
I didn't go deeper, what I see the client sends a request, but the server is silent. |
@EdwinVanVliet @nvborisenko is there something left to do in this PR? |
Converted this PR to draft, the sttaus of this PR is undetermined yet. |
@EdwinVanVliet please pay attention on #13768, probably it impacts something. |
I am closing it, the goal can be easily achieved via BCL, and TPL model specifically. Thanks for the contribution. |
Description
This PR introduces a SendCommands function to DevTools session, which is able to sent multiple Devtools commands and wait till all the commands received an answer. The singular SendCommand function will get blocked because the commands running against that target will be blocked until runtime.runIfWaitingForDebugger is invoked. By bundling the commands we can get around that issue.
This PR is a continuation of #13330
For demonstration purposes I've included a sample script.
Note that when enabling the
WaitForDebuggerOnStart
option, all targets will be halted, this includes iframes and service workers for instance. Therefore you should always issue therunIfWaitingForDebugger
command when this feature is enabled.Motivation and Context
When a new target (new tab for instance) is created DevTools continues execution based on the WaitForDebugger property set in the SetAutoAttach feature. When the WaitForDebugger property is set to false DevTools continues without waiting the consumer to properly enable all domains on the target. For instance when being interested in network events from the network domain we need to invoke Network.enable for that specific target. The current behavior causes some of the network events to be missing due to Selenium enabling the domains after the target already performed network requests.
By allowing consumers to enable the WaitForDebugger property we can enable all domains and tell DevTools whenever we are ready by using the Run.runIfWaitingForDebugger command.
Types of changes
Checklist