Skip to content

Commit

Permalink
fix(web-sockets): Use separate server for web sockets in proxy mode - f…
Browse files Browse the repository at this point in the history
…ixes #625
  • Loading branch information
shakyShane committed Jul 27, 2015
2 parents f35d034 + 1c9d929 commit 40017b4
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 15 deletions.
4 changes: 4 additions & 0 deletions lib/async-tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ module.exports = [
step: "Finding an empty port",
fn: async.getEmptyPort
},
{
step: "Getting an extra port for Proxy",
fn: async.getExtraPortForProxy
},
{
step: "Checking online status",
fn: async.getOnlineStatus
Expand Down
41 changes: 41 additions & 0 deletions lib/async.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,47 @@ module.exports = {
});
});
},
/**
* If the running mode is proxy, we'll use a separate port
* for the Browsersync web-socket server. This is to eliminate any issues
* with trying to proxy web sockets through to the users server.
* @param bs
* @param done
*/
getExtraPortForProxy: function (bs, done) {
/**
* An extra port is not needed in snippet/server mode
*/
if (bs.options.get("mode") !== "proxy") {
return done();
}

/**
* Use 1 higher than server port by default...
*/
var socketPort = bs.options.get("port") + 1;

/**
* Or use the user-defined socket.port option instead
*/
if (bs.options.hasIn(["socket", "port"])) {
socketPort = bs.options.getIn(["socket", "port"]);
}

utils.getPort(socketPort, null, function (err, port) {
if (err) {
return done(err);
}
done(null, {
optionsIn: [
{
path: ["socket", "port"],
value: port
}
]
});
});
},
/**
* Some features require an internet connection.
* If the user did not provide either `true` or `false`
Expand Down
7 changes: 6 additions & 1 deletion lib/cli/command.reload.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ module.exports = function (opts) {
}
var proto = require("../http-protocol");
var scheme = flags.url.match(/^https/) ? "https" : "http";
var args = {method: "reload"};

var url = proto.getUrl({method: "reload", args: flags.files}, flags.url);
if (flags.files) {
args.args = flags.files;
}

var url = proto.getUrl(args, flags.url);

require(scheme).get(url, function (res) {
res.on("data", function () {
Expand Down
9 changes: 7 additions & 2 deletions lib/connect-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,20 @@ var connectUtils = {
var withHostnamePort = "'{protocol}' + location.hostname + ':{port}{ns}'";
var withHost = "'{protocol}' + location.host + '{ns}'";
var withDomain = "'{domain}{ns}'";
var port = options.get("port");

// default use-case is server/proxy
var string = withHost;

if (options.get("mode") === "snippet") {
if (options.get("mode") !== "server") {
protocol = options.get("scheme") + "://";
string = withHostnamePort;
}

if (options.get("mode") === "proxy") {
port = options.getIn(["socket", "port"]);
}

if (socketOpts.domain) {
string = withDomain;
if (typeof socketOpts.domain === "function") {
Expand All @@ -109,7 +114,7 @@ var connectUtils = {

return string
.replace("{protocol}", protocol)
.replace("{port}", options.get("port"))
.replace("{port}", port)
.replace("{domain}", socketOpts.domain)
.replace("{ns}", namespace);
},
Expand Down
4 changes: 4 additions & 0 deletions lib/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,11 +378,15 @@ module.exports = {
* @param {String} [clientPath="/browser-sync"]
* @param {String|Function} [namespace="/browser-sync"]
* @param {String|Function} [domain=undefined]
* @param {String|Function} [port=undefined]
* @param {Object} [clients.heartbeatTimeout=5000]
* @since 1.6.2
* @type Object
*/
socket: {
socketIoOptions: {
log: false,
},
path: "/browser-sync/socket.io",
clientPath: "/browser-sync",
namespace: "/browser-sync",
Expand Down
4 changes: 2 additions & 2 deletions lib/server/proxy-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ module.exports = function createProxyServer (bs, scripts) {
bs.proxy.app.use(bs.options.getIn(["scriptPaths", "path"]), scripts);

/**
* How best to handle websockets going forward?
* Also proxy upgrades for Web Socket support
*/
//proxy.server.on("upgrade", app.handleUpgrade);
proxy.server.on("upgrade", bs.proxy.app.handleUpgrade);

return proxy;
};
Expand Down
21 changes: 18 additions & 3 deletions lib/sockets.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"use strict";

var socket = require("socket.io");
var utils = require("./server/utils");
var Steward = require("emitter-steward");

/**
Expand All @@ -18,10 +19,24 @@ module.exports.plugin = function (server, clientEvents, bs) {
*/
module.exports.init = function (server, clientEvents, bs) {

var emitter = bs.events;
var socketConfig = bs.options.get("socket").toJS();
var emitter = bs.events;

var io = socket.listen(server, {log: false, path: socketConfig.path});
var socketConfig = bs.options
.get("socket")
.toJS();

if (bs.options.get("mode") === "proxy") {
server = utils.getServer(null, bs.options).server;
server.listen(bs.options.getIn(["socket", "port"]));
bs.registerCleanupTask(function () {
server.close();
});
}

var socketIoConfig = socketConfig.socketIoOptions;
socketIoConfig.path = socketConfig.path;

var io = socket(server, socketIoConfig);

// Override default namespace.
io.sockets = io.of(socketConfig.namespace);
Expand Down
3 changes: 3 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ var utils = {
max = ports.get("max") || null;
}

utils.getPort(port, max, cb);
},
getPort: function (port, max, cb) {
portScanner.findAPortNotInUse(port, max, {
host: "localhost",
timeout: 1000
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,23 @@
"async-each-series": "^0.1.1",
"browser-sync-client": "^2.2.1",
"browser-sync-ui": "^0.5.12",
"chokidar": "^1.0.4",
"chokidar": "^1.0.5",
"connect": "^3.4.0",
"dev-ip": "^1.0.1",
"easy-extender": "^2.3.1",
"eazy-logger": "^2.1.2",
"emitter-steward": "^0.0.1",
"foxy": "^11.1.1",
"immutable": "^3.7.4",
"localtunnel": "^1.6.0",
"localtunnel": "^1.7.0",
"lodash": "^3.9.3",
"longest": "^1.0.1",
"meow": "^3.3.0",
"opn": "^2.0.1",
"pad-left": "^1.0.2",
"portscanner": "^1.0.0",
"query-string": "^2.3.0",
"resp-modifier": "^4.0.3",
"query-string": "^2.4.0",
"resp-modifier": "^4.0.4",
"serve-index": "^1.7.0",
"serve-static": "^1.10.0",
"socket.io": "^1.3.6",
Expand All @@ -75,7 +75,7 @@
"istanbul-coveralls": "^1.0.3",
"mocha": "^2.2.5",
"q": "^1.4.1",
"request": "^2.59.0",
"request": "^2.60.0",
"sinon": "^1.15.4",
"slugify": "^0.1.1",
"socket.io-client": "^1.3.6",
Expand Down
51 changes: 51 additions & 0 deletions test/specs/e2e/e2e.options.port.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,55 @@ describe("E2E `port` option", function () {
done();
});
});
it("sets extra port option for socket in proxy mode", function (done) {
browserSync.reset();

var stub = sinon.stub(utils, "getPort");

stub.onCall(0).yields(null, 3000);
stub.onCall(1).yields(null, 3001);

var config = {
logLevel: "silent",
proxy: "localhost",
online: false,
open: false
};

browserSync(config, function (err, bs) {
bs.cleanup();
assert.equal(bs.options.get("port"), 3000);
assert.equal(stub.getCall(1).args[0], 3001);
assert.equal(bs.options.getIn(["socket", "port"]), 3001);
utils.getPort.restore();
done();
});
});
it("uses user-given extra port option for socket in proxy mode", function (done) {
browserSync.reset();

var stub = sinon.stub(utils, "getPort");

stub.onCall(0).yields(null, 3000);
stub.onCall(1).yields(null, 8001);

var config = {
logLevel: "silent",
proxy: "localhost",
socket: {
port: 8001
},
online: false,
open: false
};

browserSync(config, function (err, bs) {
bs.cleanup();
assert.equal(bs.options.get("port"), 3000);
assert.equal(stub.getCall(1).args[0], 8001);
assert.equal(bs.options.getIn(["socket", "port"]), 8001);
utils.getPort.restore();
done();
});
});
});
7 changes: 5 additions & 2 deletions test/specs/utils/utils.connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@ describe("Connection utils", function () {
var options = merge({
port: 3002,
scheme: "http",
mode: "proxy"
mode: "proxy",
socket: {
port: 4000
}
});
var actual = utils.socketConnector(options);
assert.include(actual, "'' + location.host + '/browser-sync'");
assert.include(actual, "'http://' + location.hostname + ':4000/browser-sync'");
});
it("should return a connection url for server mode", function () {
var options = merge({
Expand Down

0 comments on commit 40017b4

Please sign in to comment.