Closed Bug 1411207 Opened 7 years ago Closed 1 year ago

WebDriver:NewSession hangs when proxy configuration is duplicated in capabiliites

Categories

(Remote Protocol :: Marionette, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
87 Branch

People

(Reporter: skeletor, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: hang)

Attachments

(1 file)

Attached file test.diff
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36

Steps to reproduce:

While working on Bug 1395886, I added a test to test_proxy.py and made some changes to session.js; the changes are in the attached diff. I then tried running the tests in test_proxy.py. I'm using 64 bit Windows 10.


Actual results:

When I tried to run the new test it caused the Marionette server to hang.


Expected results:

As it stands the tests should not pass, but the server should not hang.
What do you mean by that it hangs?  What are the reproduction steps?
Attachment #8921408 - Attachment mime type: text/x-patch → text/plain
I can see this hang with the attached patch. I will investigate but I think it's due to the unhandled exception. Basically we should never hang based on that, but always see a failed response.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: hang
Priority: -- → P1
The hang happens because proxy.fromJSON() is called a second time, and then it fails:

1508948384099	Marionette	TRACE	4 -> [0,1,"newSession",{"proxy":{"proxyType":"manual","ftpProxy":"username:password@marionette.test:21"},"capabilities":{"proxy":{"proxyType":"manual","ftpProxy":"username:password@marionette.test:21"}}}]
1508948384102	Marionette	DEBUG	Adding login for ftp://marionette.test
1508948384141	Marionette	INFO	Proxy settings initialised: {"proxyType":"manual","ftpProxy":"username:password@marionette.test:21"}
1508948384151	Marionette	DEBUG	Register listener.js for window 2147483649
1508948384172	Marionette	DEBUG	Adding login for ftp://marionette.test
JavaScript error: file:///Volumes/data/code/gecko/obj/debug/dist/NightlyDebug.app/Contents/Resources/components/nsLoginManager.js, line 384: TypeError: this._storage is null

The first call correctly sets up the proxy authentication! But then we get this strange Javascript error. Given that only a single proxy host has been specified, we should not run this code at all.

More investigation has shown that this is actually the code in listener.js which fetches the session capabilities:

https://dxr.mozilla.org/mozilla-central/rev/a124f4901430f6db74cfc7fe3b07957a1c691b40/testing/marionette/listener.js#594

And it's clear now why this is busted! The loginmanager is not available from the frame script.

Andreas, as it looks like we really have to refactor this code, or have to make sure that fromJSON is not modifying any preferences and really only return the actual object, without calling init().

Anyway, we should never fail and hang when an unexpected exception is raised in the listener.
Priority: P1 → P2
OS: Unspecified → All
Priority: P2 → P3
Hardware: Unspecified → All
Version: 58 Branch → Trunk
Blocks: webdriver
Priority: P3 → P2
Summary: Marionette server hangs during testing → WebDriver:NewSession hangs when proxy configuration is duplicated in capabiliites
Severity: normal → S3

With getting rid of the framescripts in Marionette (bug 1669172) for Firefox 87 this extra call to get capabilities doesn't exist anymore. By quickly applying the patch and making small fixes to merge conflicts I can verify that this no longer hangs.

Ian, it's been a long time since you filed the bug but I wonder if you would be interested to continue on that patch - in case you are still working in this area and have interest to see the proxy authentication code.

Status: NEW → RESOLVED
Closed: 1 year ago
Depends on: 1669172
Flags: needinfo?(rubberdonkeysandwich)
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: