benderjs / browser-launcher2

☠ This package is no longer maintained ☠
Other
24 stars 13 forks source link

instance.stop() killing all my (other) Chrome tabs #20

Closed alessioalex closed 9 years ago

alessioalex commented 9 years ago

Hey,

On Windows 7 when I call instance.stop() all my other instances are killed.

I also experience another strange thing, I see logs telling me more browsers that I've launched are stopped:

ie started with PID: 12536
ie (PID: 12536) stopped with exit code: 0
chrome started with PID: 8284
ie (PID: 12536) stopped with exit code: 1
chrome started with PID: 8384
chrome (PID: 8384) stopped with exit code: 0
firefox started with PID: 10460
chrome (PID: 8284) stopped with exit code: 0
firefox started with PID: 10804
chrome (PID: 8384) stopped with exit code: 1
chrome (PID: 8284) stopped with exit code: 1
firefox started with PID: 4736
firefox started with PID: 2892
firefox (PID: 10460) stopped with exit code: 0
firefox (PID: 4736) stopped with exit code: 1
firefox (PID: 10804) stopped with exit code: 1
firefox (PID: 2892) stopped with exit code: 1
firefox (PID: 10460) stopped with exit code: 1

My code looks like this:

var launcher = require( '../' );
var BROWSER = process.env.BROWSER || 'chrome';

launcher(function(err, launch) {
  if (err) {
    return console.error( err );
  }

  console.log('# available browsers:');
  console.dir(launch.browsers);

  var go = launchBrowser.bind(null, launch);

  go('ie', function onStop() {
    go('chrome', function onStop() {
      go('firefox');
    });
  });
});

function launchBrowser(launch, browser, cb) {
  cb = cb || function noop() {};

  launch('http://cksource.com/', browser, function(err, instance) {
    if (err) {
      return console.error( err );
    }

    setTimeout(function() {
      instance.stop();
    }, 10000);

    console.log('%s started with PID:', browser, instance.pid);

    instance.on('stop', function(code) {
      console.log('%s (PID: %s) stopped with exit code:', browser, instance.pid, code);
      cb();
    });
  });
}
alessioalex commented 9 years ago

I'm not sure of all the implications, but in my case using the native Node child_process.kill() worked:

    //...
    setTimeout(function() {
      instance.process.kill();
    }, 10000);
   //...

@gregpabian do you want me to make a pull request on this or is there something I missed?

OS: Windows 7 Node: 0.10.22 NPM: 1.3.14

I also think this function should work on other OSes. I can test on OSX on my end when I get back home. Let me know.

gregpabian commented 9 years ago

On Windows 7 when I call instance.stop() all my other instances are killed.

This can be easily fixed for most of the browsers, but there's still a problematic case, see below.

I'm not sure of all the implications, but in my case using the native Node child_process.kill() worked:

Let me first explain why we're not using process.kill() right now.

Most of the browsers available on Windows can be launched using their executable - e.g. chrome.exe, firefox.exe or iexplore.exe, but then there is Opera... To launch the Opera browser properly the Launcher.exe file is used. That file start the actual instance of the browser (Opera.exe) and then stops immediately. It means that:

So I think we may use process.kill for most of the browsers, but still have to work out a better solution for Opera-.

I also experience another strange thing, I see logs telling me more browsers that I've launched are stopped

I still have to investigate it, but thanks.

do you want me to make a pull request on this or is there something I missed?

I'll take care of it once I find a better approach for Opera-like cases.

Anyway, thanks again for reporting this.