GCuser99 / SeleniumVBA

A comprehensive Selenium wrapper for browser automation developed for MS Office VBA running in Windows
MIT License
83 stars 17 forks source link

Browser doesn't change if driver closed unexpectedly #70

Closed 6DiegoDiego9 closed 1 year ago

6DiegoDiego9 commented 1 year ago

Browser doesn't change if driver closed without a proper .Shutdown on last execution, for example in case you Reset VBE just after an error. This bug has been happening for a long time and is of low importance to me but I report it, just in case you've a quick solution. I didn't look into it yet.

To reproduce the problem, run test1, then test2: you'll find that a second Edge browser opens instead than Chrome.

Private Sub test1()
    Dim driver As New WebDriver
    driver.StartEdge
    driver.OpenBrowser
End Sub

Private Sub test2()
    Dim driver As New WebDriver
    driver.StartChrome
    driver.OpenBrowser
End Sub

By the way, I just found a way to to intercept a VBE Reset and launch a (cleaner/killer) macro when it happens, but I see that it doesn't trigger if Reset is done while in debugging mode so it's of very little use here.

GCuser99 commented 1 year ago

Yeah, we are currently using the same local host port for Edge and Chrome, and since we are not killing the stranded driver for Edge/Chrome on Start in order to preserve multi-session support, your observation will occur (see test_MultiSession). One solution would be to use 9515 for Chrome default, and 9516 for Edge default. Another way is to kill stranded driver on Start, like we must do with Firefox and IE, but that would have the effect of forcing user to use different ports for same [Edge or Chrome] browser in a multi-session usage case. I do think we should fix this. I'm leaning toward the different default ports. What do you think?

Anyway, try below and you will see that you get Chrome on the second test.

Private Sub test1()
    Dim driver As New WebDriver
    driver.StartEdge , 9515
    driver.OpenBrowser
End Sub

Private Sub test2()
    Dim driver As New WebDriver
    driver.StartChrome , 9516
    driver.OpenBrowser
End Sub
6DiegoDiego9 commented 1 year ago

The .Shutdown method already doesn't manage having multiple driver objects running on a single driverprocess:port, as in this script:

Private Sub test1()
    Dim driver1 As New WebDriver
    Dim driver2 As New WebDriver
    driver1.StartEdge , 9515
    driver2.StartEdge , 9515
    driver1.OpenBrowser
    driver2.OpenBrowser
    driver1.NavigateTo "https://www.google.it"
    driver2.NavigateTo "https://www.google.com"
    driver1.Shutdown
    driver2.Shutdown  'error happens here
End Sub

This is because it kills the msedgedriver.exe process already on driver1.Shutdown.

I also read in your multisession test that using the same port "should work with only limited interference" and "for logging we only get one log (the first one)".

Are you sure that we must support running multiple objects/browsers instances running on the same driverprocess:port? is this officially supported? Otherwise we must also enforce the different ports (and separate exe processes), in addition to set different default values...

GCuser99 commented 1 year ago

The .Shutdown method already doesn't manage having multiple driver objects running on a single driverprocess:port, as in this script:

Private Sub test1()
    Dim driver1 As New WebDriver
    Dim driver2 As New WebDriver
    driver1.StartEdge , 9515
    driver2.StartEdge , 9515
    driver1.OpenBrowser
    driver2.OpenBrowser
    driver1.NavigateTo "https://www.google.it"
    driver2.NavigateTo "https://www.google.com"
    driver1.Shutdown
    driver2.Shutdown  'error happens here
End Sub

Yes, I agree. What I believe is going on in this script is that for Edge/Chrome, if you start an additional driver on same port as a previous one, you really only get one driver process, not two. But because you have two SeleniumVBA WebDriver instances, with their own OpenBrowser session ID's and other associated settings, it works correctly because Edge/Chrome support multi-sessions. The double Shutdown is problematic of course, because there is really only one driver process, not two. I do think this is officially supported (multi-sessions) but our implementation is lacking with the Shutdown issue.

Anyway, let's continue to work this - I'll do some more research and testing when I get time. Thanks!

GCuser99 commented 1 year ago

BTW, on the dual Edge-Chrome drivers issue of your original post above, I think we should change the port of Edge to 9516. I also think it would be a good idea to add default-override port specification to the ini file, in case user is already using any of the default driver ports for another application.

GCuser99 commented 1 year ago

I'm closing this for now - implemented changing the MS Edge port to 9516 several versions ago. Still some potential issues with multi-session support but will address further when it's a problem...