Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Server.attach method #213

Merged
merged 5 commits into from Jun 4, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 38 additions & 5 deletions README.md
Expand Up @@ -120,6 +120,30 @@ These are exposed by `require('engine.io')`:

##### Methods

- `()`
- Returns a new `Server` instance. If the first argument is an `http.Server` then the
new `Server` instance will be attached to it. Otherwise, the arguments are passed
directly to the `Server` constructor.
- **Parameters**
- `http.Server`: optional, server to attach to.
- `Object`: optional, options object (see `Server#constructor` api docs below)

The following are identical ways to instantiate a server and then attach it.
```js
var httpServer; // previously created with `http.createServer();` from node.js api.

// create a server first, and then attach
var eioServer = require('engine.io').Server();
eioServer.attach(httpServer);

// or call the module as a function to get `Server`
var eioServer = require('engine.io')();
eioServer.attach(httpServer);

// immediately attach
var eioServer = require('engine.io')(http_server);
```

- `listen`
- Creates an `http.Server` which listens on the given port and attaches WS
to it. It returns `501 Not Implemented` for regular http requests.
Expand All @@ -134,11 +158,9 @@ These are exposed by `require('engine.io')`:
- `http.Server`: server to attach to.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be optional. We need to document the various ways this object can be instantiated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this optional for the attach method? If you don't provide something to attach to... then attach can't do anything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean for constructing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, But that would be documented under the Server constructor docs right? For this attach call it is required. At least that is how the code currently is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right I thought I was commenting on the ctor docs

- `Object`: optional, options object
- **Options**
- `path` (`String`): name of the path to capture (`/engine.io`).
- `destroyUpgrade` (`Boolean`): destroy unhandled upgrade requests (`true`)
- `destroyUpgradeTimeout` (`Number`): milliseconds after which unhandled requests are ended (`1000`)
- **See Server options below for additional options you can pass**
- **Returns** `Server`
- All options from `Server.attach` method, documented below.
- **Additionally** See Server `constructor` below for options you can pass for creating the new Server
- **Returns** `Server` a new Server instance.

<hr><br>

Expand Down Expand Up @@ -205,6 +227,17 @@ to a single process.
- `net.Stream`: TCP socket for the request
- `Buffer`: legacy tail bytes
- **Returns** `Server` for chaining
- `attach`
- Attach this Server instance to an `http.Server`
- Captures `upgrade` requests for a `http.Server`. In other words, makes
a regular http.Server WebSocket-compatible.
- **Parameters**
- `http.Server`: server to attach to.
- `Object`: optional, options object
- **Options**
- `path` (`String`): name of the path to capture (`/engine.io`).
- `destroyUpgrade` (`Boolean`): destroy unhandled upgrade requests (`true`)
- `destroyUpgradeTimeout` (`Number`): milliseconds after which unhandled requests are ended (`1000`)

<hr><br>

Expand Down
4 changes: 2 additions & 2 deletions index.js
@@ -1,4 +1,4 @@

module.exports = process.env.EIO_COV
? require('./lib-cov')
: require('./lib');
? require('./lib-cov/engine.io')
: require('./lib/engine.io');
84 changes: 32 additions & 52 deletions lib/engine.io.js
Expand Up @@ -2,8 +2,31 @@
* Module dependencies.
*/

var http = require('http')
, debug = require('debug')('engine:core');
var http = require('http');

/**
* Invoking the library as a function delegates to attach if the first argument
* is an `http.Server`.
*
* If there are no arguments or the first argument is an options object, then
* a new Server instance is returned.
*
* @param {http.Server} server (if specified, will be attached to by the new Server instance)
* @param {Object} options
* @return {Server} engine server
* @api public
*/

exports = module.exports = function() {
// backwards compatible use as `.attach`
// if first argument is an http server
if (arguments.length && arguments[0] instanceof http.Server) {
return attach.apply(this, arguments);
}

// if first argument is not an http server, then just make a regular eio server
return exports.Server.apply(null, arguments);
};

/**
* Protocol revision number.
Expand Down Expand Up @@ -63,7 +86,9 @@ exports.parser = require('engine.io-parser');
* @api public
*/

exports.listen = function (port, options, fn) {
exports.listen = listen;

function listen(port, options, fn) {
if ('function' == typeof options) {
fn = options;
options = {};
Expand Down Expand Up @@ -92,55 +117,10 @@ exports.listen = function (port, options, fn) {
* @api public
*/

exports.attach = function (server, options) {
var engine = new exports.Server(options);
var options = options || {};
var path = (options.path || '/engine.io').replace(/\/$/, '');

var destroyUpgrade = (options.destroyUpgrade !== undefined) ? options.destroyUpgrade : true;
var destroyUpgradeTimeout = options.destroyUpgradeTimeout || 1000;

// normalize path
path += '/';

function check (req) {
return path == req.url.substr(0, path.length);
}

// cache and clean up listeners
var listeners = server.listeners('request').slice(0);
server.removeAllListeners('request');
server.on('close', engine.close.bind(engine));

// add request handler
server.on('request', function(req, res){
if (check(req)) {
debug('intercepting request for path "%s"', path);
engine.handleRequest(req, res);
} else {
for (var i = 0, l = listeners.length; i < l; i++) {
listeners[i].call(server, req, res);
}
}
});

if(~engine.transports.indexOf('websocket')) {
server.on('upgrade', function (req, socket, head) {
if (check(req)) {
engine.handleUpgrade(req, socket, head);
} else if (false !== options.destroyUpgrade) {
// default node behavior is to disconnect when no handlers
// but by adding a handler, we prevent that
// and if no eio thing handles the upgrade
// then the socket needs to die!
setTimeout(function() {
if (socket.writable && socket.bytesWritten <= 0) {
return socket.end();
}
}, options.destroyUpgradeTimeout);
}
});
}
exports.attach = attach;

function attach(server, options) {
var engine = new exports.Server(options);
engine.attach(server, options);
return engine;
};
25 changes: 0 additions & 25 deletions lib/index.js

This file was deleted.

63 changes: 63 additions & 0 deletions lib/server.js
Expand Up @@ -28,6 +28,10 @@ module.exports = Server;
*/

function Server(opts){
if (!(this instanceof Server)) {
return new Server(opts);
}

this.clients = {};
this.clientsCount = 0;

Expand Down Expand Up @@ -314,3 +318,62 @@ Server.prototype.onWebSocket = function(req, socket){
this.handshake(req._query.transport, req);
}
};

/**
* Captures upgrade requests for a http.Server.
*
* @param {http.Server} server
* @param {Object} options
* @api public
*/

Server.prototype.attach = function(server, options){
var self = this;
var options = options || {};
var path = (options.path || '/engine.io').replace(/\/$/, '');

var destroyUpgrade = (options.destroyUpgrade !== undefined) ? options.destroyUpgrade : true;
var destroyUpgradeTimeout = options.destroyUpgradeTimeout || 1000;

// normalize path
path += '/';

function check (req) {
return path == req.url.substr(0, path.length);
}

// cache and clean up listeners
var listeners = server.listeners('request').slice(0);
server.removeAllListeners('request');
server.on('close', self.close.bind(self));

// add request handler
server.on('request', function(req, res){
if (check(req)) {
debug('intercepting request for path "%s"', path);
self.handleRequest(req, res);
} else {
for (var i = 0, l = listeners.length; i < l; i++) {
listeners[i].call(server, req, res);
}
}
});

if(~self.transports.indexOf('websocket')) {
server.on('upgrade', function (req, socket, head) {
if (check(req)) {
self.handleUpgrade(req, socket, head);
} else if (false !== options.destroyUpgrade) {
// default node behavior is to disconnect when no handlers
// but by adding a handler, we prevent that
// and if no eio thing handles the upgrade
// then the socket needs to die!
setTimeout(function() {
if (socket.writable && socket.bytesWritten <= 0) {
return socket.end();
}
}, options.destroyUpgradeTimeout);
}
});
}
};
14 changes: 14 additions & 0 deletions test/engine.io.js
Expand Up @@ -26,6 +26,13 @@ describe('engine', function () {
expect(version).to.be(require('engine.io-client/package').version);
});

describe('engine()', function () {
it('should create a Server when require called with no arguments', function () {
var engine = eio();
expect(engine).to.be.an(eio.Server);
});
});

describe('listen', function () {
it('should open a http server that returns 501', function (done) {
var server = listen(function (port) {
Expand All @@ -38,6 +45,13 @@ describe('engine', function () {
});

describe('attach()', function () {
it('should work from require()', function () {
var server = http.createServer();
var engine = eio(server);

expect(engine).to.be.an(eio.Server);
});

it('should return an engine.Server', function () {
var server = http.createServer()
, engine = eio.attach(server);
Expand Down