Open Bug 1421766 Opened 2 years ago Updated 9 days ago

Read Marionette port from profile preference (was: No connection to Marionette can be made due to unknown port for custom profile)

Categories

(Testing :: geckodriver, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

(Blocks 2 open bugs, )

Details

As isolated today from https://github.com/mozilla/geckodriver/issues/1058 there is a problem with the Marionette connection because the port marionette creates the server socket on is not known to geckodriver. Instead geckodriver tries to connect to a port it automatically got when checking for a free port. The result is a connection timeout after those 600 tries.

To allow geckodriver to connect to Marionette for the given instance of Firefox, several ways would be possible. After a team discussion today, we agreed on the following steps:

1. geckodriver continuous to find a free port by querying via port 0

2. If a custom profile has been specified via `args` inside the `moz:firefoxOptions` capability, the path of the profile has to be extracted. 

2.1. Inside the profile open the file `prefs.js` and replace the value of the preference `marionette.port` with the above retrieved port number. Store the changes afterward.

3. Continue to start Firefox with the profile

With step 2 Marionette will know which port to use, when starting the server socket. Once that is done, also geckodriver can connect to the known port.
Priority: -- → P1
Given that there is a workaround for the problem we can lower the priority to P2. But it would be still great to have this fixed for the next geckodriver release.
Blocks: 1401129
Priority: P1 → P2
I feel like the way this was supposed to work was by adding the preference to the profile passed in. Do we not do that?
Oh, right, maybe not when it's passed in as an argument rather than as a zip file.
Yes, I believe we skip setting marionette.port when the profile
path is given.  We will still want to set the “override” prefs,
marionette.port and marionette.log.level.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Andreas, will you fix that or should I take this bug? I would like to get it shipped with the next release soonish.
Flags: needinfo?(ato)
This is quite a hard bug to fix because we pass off ownership of the
profile to rust_mozrunner.  We somehow need to pass the ownership back
to geckodriver so it can mutate it, then back to rust_mozrunner again.
I would much prefer changing rust_mozrunner’s API altogether,
but that requires upstream changes because it does not live in tree.

In any case I’m happy for you to try to figure this out.
Assignee: ato → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(ato)
Depends on: 1431459
Priority: P2 → P3
Version: 59 Branch → unspecified
No longer blocks: 1401129
Blocks: 1466818
No longer blocks: 1441204
No longer blocks: 1466818
Blocks: 1491288
I have made a patch that fixes the problem like described in the first post.

This was my first time of programming in rust, so i dont know if my code is very elegant but at least it works for me...

diff -c -r geckodriver-0.21.0/src/marionette.rs geckodriver-0.21.0-patched/src/marionette.rs
*** geckodriver-0.21.0/src/marionette.rs        Fri Jun 15 22:46:25 2018
--- geckodriver-0.21.0-patched/src/marionette.rs        Fri Sep 14 18:28:47 2018
***************
*** 53,58 ****
--- 53,61 ----
  use capabilities::{FirefoxCapabilities, FirefoxOptions};
  use logging;
  use prefs;
+ use regex::Regex;
+ use regex::NoExpand;
+ use std::fs;

  // localhost may be routed to the IPv6 stack on certain systems,
  // and nsIServerSocket in Marionette only supports IPv4
***************
*** 471,476 ****
--- 474,489 ----
              runner.arg("-jsdebugger");
          }
          if let Some(args) = options.args.as_ref() {
+           let mut found = false;
+           for i in 0..args.len() {
+               if found {
+                   try!(self.update_port_in_profile(port, &args[i]));
+                   break;
+               }
+               if args[i] == "-profile" {
+                   found = true;
+               }
+           }
              runner.args(args);
          }

***************
*** 484,489 ****
--- 497,516 ----

          Ok(())
      }
+
+     fn update_port_in_profile(&mut self, port: u16, profile_dir: &String) -> WebDriverResult<()>
+     {
+       let mut filename: String = profile_dir.to_owned();
+       filename.push_str("/prefs.js");
+       let content = fs::read_to_string(&filename).expect("Unable to read prefs.js");
+       let re = Regex::new(r#"marionette.port", [0-9]*"#).unwrap();
+       let tmp = format!(r#"marionette.port", {}"#, port);
+       let result = re.replace(&content, NoExpand(&tmp));
+       let mut file = File::create(&filename).expect("Unable to open prefs.js");
+       file.write_all(result.as_bytes()).expect("Unable to write to prefs.js");
+
+         Ok(())
+     }

      pub fn set_prefs(&self, port: u16, profile: &mut Profile, custom_profile: bool,
                       extra_prefs: Vec<(String, Pref)>)
Hi Avast. Thank you for the proposal in how to get this bug fixed. Generally we only accept real patch files, and as such you would have to create the patch based on the latest state in mozilla-central. Please see the following link for details:

https://firefox-source-docs.mozilla.org/testing/geckodriver/geckodriver/index.html#for-developers

Otherwise I want to refer to comment 7 of this bug, which outlines how a proper solution to fix the underlying issue should be done.

So would you have a bit of time to work with Andreas and get a patch completed?
Flags: needinfo?(avast)
Hello Hendrik, sorry for my stupid name, i have entered the real name in my profile now.

I possibly do not understand how geckodriver and firefox work together, but as far as i understand it is useless to modify the profile that is created by geckodriver and passed to firefox, because firefox is started with a completely other profile.

I am using the perl bindings and the following script:

$driver = Selenium::Remote::Driver->new(
    'extra_capabilities' => {
        'moz:firefoxOptions' => {
            'args' => [ "-profile", "$ENV{'HOME'}/.mozilla/firefox/default" ],
        },
    }
);

creates a folder /tmp/rust_mozprofile.... which contains a user.js which in turn contains a line "user_pref("marionette.port", 33573);" but firefox is started with "/usr/bin/firefox" "-marionette" "-profile" "/home/horn/.mozilla/firefox/default" and the prefs.js in /home/horn/.mozilla/firefox/default contains the line "user_pref("marionette.port", 2828);"

So i personally think the solution described in the first post of this bug is correct here and i tried to implement that.

of course one can discuss if this way of passing the marionette port-number is optimal at all. I personally believe that firefox should determine a free port number and open a socket on that port and then pass the portnumber to geckodriver who then can connect to the port. possibly this can simply be done by making firefox print the portnumber to stdout and geckodriver read that portnumber from a pipe. But this is all speculation from my side...
Flags: needinfo?(avast)
From what you describe, I would not expect the temporary profile
to be created, although maybe we are creating that unconditionally?
That would seem like a separate bug.

It then correctly recognises the /home/horn/.mozilla/firefox/default
profile you pass by moz:firefoxOptions.  The problem with this
approach is that geckodriver does not know how to write the Marionette
port number to this profile.  2828 is the default port number, but
geckodriver assigns a random port number so you can operate on
multiple Firefoxen simultaneously.  It is obviously a bug that we
don’t support custom profiles yet.

You may have greater luck launching geckodriver like this: "geckodriver
--marionette-port 2828" as that forces the port to be the default
port already defined in /home/horn/.mozilla/firefox/default, and
as geckodriver passes the -marionette flag to Firefox, this should
let you connect and use the custom profile.

IHTH
Yes, the temporary profile is created also if "-profile" is given in args.

> It then correctly recognises the /home/horn/.mozilla/firefox/default
> profile you pass by moz:firefoxOptions. The problem with this
> approach is that geckodriver does not know how to write the Marionette
> port number to this profile.

That's what my patch does and what was proposed in the description of this bug, it opens the file prefs.js in /home/horn/.mozilla/firefox/default and replaces the port number with the current random port number.

This also works with multiple firefox instances at one time if your test script provides a separate copy of the original profile folder for each instance it starts. I do that like this:

$moz_prof_dir = "$ENV{'HOME'}/.mozilla/firefox/profile.$$";
system("cp -a $ENV{'HOME'}/.mozilla/firefox/default $moz_prof_dir");
$driver = Selenium::Remote::Driver->new(
  extra_capabilities => {
    'moz:firefoxOptions' => {
        args => [ "-profile", $moz_prof_dir ],
    }
  }
);

Works perfectly for me. If geckodriver creates the profile folder, it too creates one for each firefox session and writes the port number to the file user.js. If you provide the profile by yourself, you tell geckodriver where this profile lives and it just has to insert the proper port number into that profile instead of creating it completely.

> You may have greater luck launching geckodriver like this: "geckodriver
> --marionette-port 2828" as that forces the port to be the default
> port already defined in /home/horn/.mozilla/firefox/default

That's the workaround that was proposed above and it works indeed, but it does NOT work with multiple firefoxes simultaneously.
Blocks: 1495062
No longer blocks: 1491288
Duplicate of this bug: 1517249
Summary: No connection to Marionette can be made due to unknown port for custom profile → Read Marionette port from profile preference (was: No connection to Marionette can be made due to unknown port for custom profile)

We won't be able to fix that for the 0.24 release. Moving out to 0.25.

Blocks: 1520585
No longer blocks: 1495062
No longer blocks: 1520585
You need to log in before you can comment on or make changes to this bug.