Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

add Server.attach method #213

Merged
merged 5 commits into from

3 participants

@defunctzombie
Collaborator

This method does what engine.attach use to do but is now on the
instantiated server instance. This makes it possible to create engine.io
servers without yet attaching them to any active http server.

When creating a project which will server static files/pages alongside
engine.io on the same domain/process this makes it easier to separate
logic into files without having to pass around the http server.

@rauchg
Owner

Docs plz

@defunctzombie
Collaborator

Where do the docs live?

@rauchg rauchg commented on the diff
README.md
@@ -134,12 +134,9 @@ These are exposed by `require('engine.io')`:
- `http.Server`: server to attach to.
@rauchg Owner
rauchg added a note

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

@defunctzombie Collaborator

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

@rauchg Owner
rauchg added a note

I mean for constructing.

@defunctzombie Collaborator

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.

@rauchg Owner
rauchg added a note

Right I thought I was commenting on the ctor docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@defunctzombie
Collaborator

Added some more documentation about this to the readme as well as which methods I felt are [Recommended] vs which are not.

@rauchg
Owner
@defunctzombie
Collaborator
@rauchg
Owner

That's a really good point. In this case I think there's something to be said about the convenience of being able to require('engine.io')(server) and be done with things tho.

@rauchg
Owner

Like you don't need a reference to the module itself, you just want the io isntance with the attached http server.

@defunctzombie
Collaborator
@rauchg
Owner

We need to update the README to use the shorter syntax whenever possible.

@defunctzombie
Collaborator

@guille which syntax? `require('engine.io')(http_server) ?

I like the var eio = require('engine.io')(); eio.attach(http_server) approach for the flexibility it provides in having a server object before attaching.

Let me know what format is desired and I will update the readme.

@defunctzombie
Collaborator

Is there still something to do on this PR?

@rauchg
Owner

Yes. We gotta remove the "recommendations" from the README.

@defunctzombie
Collaborator

Updated the PR. Removed the Recommended words.

README.md
((4 lines not shown))
+- `handleSocket`
+ - Called with raw TCP sockets from http requests to intercept flash policy
+ file requests
+ - **Parameters**
+ - `net.Stream`: TCP socket on which requests are listened
+ - **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`).
+ - `policyFile` (`Boolean`): whether to handle policy file requests (`true`)
@rase- Owner
rase- added a note

policyFile isn't here anymore, is it?

@defunctzombie Collaborator

good catch, fixed and removed the handleSocket docs as well since that no longer exists

@rase- Owner
rase- added a note

Perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
defunctzombie added some commits
@defunctzombie defunctzombie add Server.attach method
This method does what engine.attach use to do but is now on the
instantiated server instance. This makes it possible to create engine.io
servers without yet attaching them to any active http server.

When creating a project which will server static files/pages alongside
engine.io on the same domain/process this makes it easier to separate
logic into files without having to pass around the http server.
d74f93e
@defunctzombie defunctzombie remove unused require d3ed841
@defunctzombie defunctzombie remove lib/index.js
Needless level of indirection which blocks actually reading the
important parts of the code.
cb2b80a
@defunctzombie defunctzombie make require('engine.io')() return a new Server instance
fixes #212
48f006f
@defunctzombie defunctzombie lib: remove unused file b836605
@rauchg rauchg merged commit 2ec3d89 into from
@rauchg
Owner

Huge simplification. Thanks @defunctzombie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 4, 2014
  1. @defunctzombie

    add Server.attach method

    defunctzombie authored
    This method does what engine.attach use to do but is now on the
    instantiated server instance. This makes it possible to create engine.io
    servers without yet attaching them to any active http server.
    
    When creating a project which will server static files/pages alongside
    engine.io on the same domain/process this makes it easier to separate
    logic into files without having to pass around the http server.
  2. @defunctzombie

    remove unused require

    defunctzombie authored
  3. @defunctzombie

    remove lib/index.js

    defunctzombie authored
    Needless level of indirection which blocks actually reading the
    important parts of the code.
  4. @defunctzombie
  5. @defunctzombie

    lib: remove unused file

    defunctzombie authored
This page is out of date. Refresh to see the latest.
View
43 README.md
@@ -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.
@@ -134,11 +158,9 @@ These are exposed by `require('engine.io')`:
- `http.Server`: server to attach to.
@rauchg Owner
rauchg added a note

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

@defunctzombie Collaborator

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

@rauchg Owner
rauchg added a note

I mean for constructing.

@defunctzombie Collaborator

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.

@rauchg Owner
rauchg added a note

Right I thought I was commenting on the ctor docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- `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>
@@ -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>
View
4 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');
View
84 lib/engine.io.js
@@ -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.
@@ -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 = {};
@@ -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;
};
View
25 lib/index.js
@@ -1,25 +0,0 @@
-
-/**
- * Module dependencies.
- */
-
-var engine = require('./engine.io');
-
-/**
- * Invoking the library as a function delegates to attach
- *
- * @param {http.Server} server
- * @param {Object} options
- * @return {Server} engine server
- * @api public
- */
-
-exports = module.exports = function() {
- return engine.attach.apply(this, arguments);
-};
-
-/**
- * Merge engine.
- */
-
-for (var k in engine) exports[k] = engine[k];
View
63 lib/server.js
@@ -28,6 +28,10 @@ module.exports = Server;
*/
function Server(opts){
+ if (!(this instanceof Server)) {
+ return new Server(opts);
+ }
+
this.clients = {};
this.clientsCount = 0;
@@ -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);
+ }
+ });
+ }
+};
View
14 test/engine.io.js
@@ -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) {
@@ -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);
Something went wrong with that request. Please try again.