Closed Bug 1713782 Opened 3 years ago Closed 3 years ago

"Delete Session" inappropriately closes the browser even with "--connect-existing" specified

Categories

(Testing :: geckodriver, defect, P3)

Default
defect

Tracking

(firefox91 fixed)

RESOLVED FIXED
91 Branch
Tracking Status
firefox91 --- fixed

People

(Reporter: whimboo, Assigned: jgraham)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Even with the --connect-existing argument set for geckodriver, the Delete Session command closes the browser. Instead only the active WebDriver session has to be closed.

Maybe the patch on bug 1705770 helps here.

Depends on: 1705770

Bug 1705770 actually doesn't fix it. A running instance is still killed when sending the following HTTP Post:

curl -H 'Content-Type: application/json' -d '{}' -X DELETE http://localhost:4444/session/7932a123-8161-7741-a270-2afcb45ddf1

Under such a condition the trace log shows the following output:

1622705090665	webdriver::server	DEBUG	-> DELETE /session/cf6aca71-4be6-d646-9696-eaf314f959fe {}
1622705090744	webdriver::server	DEBUG	Deleting session
1622705090781	geckodriver::marionette	ERROR	Failed to close browser connection: Socket is not connected (os error 57)
1622705090781	webdriver::server	DEBUG	<- 200 OK {"value":null}

Note that we never reach the Browser:close() method here.

So the actual problem here is that we force DeleteSession to actually call Marionette:Quit here:
https://searchfox.org/mozilla-central/rev/9975889f5c0d5c59bd22121a454beba774cbae71/testing/geckodriver/marionette/src/marionette.rs#22-23

Keeping it calling WebDriver:DeleteSession will make it work. But then we would no longer shutdown the browser in normal scenarios, what should be the default.

The solution might be to check which kind of browser is in use when trying to convert to the Marionette message, and call the appropriate method WebDriver:DeleteSession vs Marionette:Quit, but that would mean we have to pass-down the browser instance into MarionetteCommand and then into try_convert_to_marionette_message:

https://searchfox.org/mozilla-central/rev/9975889f5c0d5c59bd22121a454beba774cbae71/testing/geckodriver/src/marionette.rs#686-690

James, do you see a better way here?

Flags: needinfo?(james)
Blocks: 1700328
Assignee: nobody → james
Status: NEW → ASSIGNED
Flags: needinfo?(james)
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/1dc45c680665
Don't close the browser when deleting session with --connect-existing, r=webdriver-reviewers,whimboo
Blocks: 1686110
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: