Adds missing close callback.

[johanneswuerbach]

In the latest version the example in the README is broken as the close callback was never executed. This PR adds a close callback.

sauceConnectProcess.close(function () {
  console.log("Closed Sauce Connect process");
});

[vojtajina]

you don't have to register the 'close' listener at all (if there is no callback).

[vojtajina]

Also, you should make sure, that the callback in https://github.com/johanneswuerbach/sauce-connect-launcher/blob/3e30551295107cc0d531a6a656e9bc847328eb78/lib/sauce-connect-launcher.js#L186 is not called when killing the process. Eg. I call .close() and it exits with an error -> the original onStart callback would be called, that's wrong.

[johanneswuerbach]

@vojtajina right, I didn't saw that both callbacks are available in the scope. Fixed.

[vojtajina]

@johanneswuerbach I meant something different. There are two callbacks you can register with sauce-connect-launcher:

• when starting: var process = sauceConnect(opts, CALLBACK1)
• when closing: process.close(CALLBACK2) (the one you are adding)

If an error happens when closing the process, both callbacks will be called. That's not correct, only the CALLBACK2 should get called. The CALLBACK1 should only get called if it starts successfully or if it fails to start - just once.

Hope this explains it better. Also, check my comment on the outdated diff.

[johanneswuerbach]

@vojtajina CALLBACK1 can only be called once https://github.com/bermi/sauce-connect-launcher/blob/master/lib/sauce-connect-launcher.js#L107

[vojtajina]

@johanneswuerbach thanks, you are right, I missed this line!

[vojtajina]

@bermi thanks a lot!

Btw, are you guys following semver in any way ? Because these breaking changes in patch versions broke karma-sauce-launcher so I'm asking if you think it's better to set dep on fixed version of sauce-connect-launcher.

Thanks for your work ! Please ping me once this gets to NPM.

[bermi]

@vojtajina from the commit history I have to admit that unfortunately we have not adhered to semver strictly. Thanks for bringing that to our attention, we will stick to semver from hereon.

BTW, 0.1.10 was pushed 1h ago

[vojtajina]

@bermi Awesome, thanks !