combohandler - npm Package Compare versions

Comparing version 0.3.0 to 0.3.1

HISTORY.md

 Combo Handler History
 =====================
 
+0.3.1 (2013-04-24)
+------------------
+
+* Fixed cluster worker configuration via events rather than `setupMaster`
+  args. Custom `server` config now works, and receives all properties from
+  the parsed CLI arguments.
+
+* Changed all pidfile IO to be synchronous. It's not frequent enough to justify
+  the indeterminate nature of asynchronicity (which wasn't even used, anyway).
+
+* Added more tests, increasing code coverage slightly.
+
 0.3.0 (2013-04-22)
@@ -5,0 +17,0 @@ ------------------

lib/cluster/index.js

@@ -38,2 +38,7 @@ /**
 
+    this.startupTimeout = [];
+    this.closingTimeout = [];
+    this.flameouts = 0;
+    // TODO: configurable
+
     this._maybeCallback(cb);
@@ -109,62 +114,68 @@ }
 ComboCluster.prototype._bindCluster = function () {
-    // TODO: configurable
-    var startupTimeout = [],
-        closingTimeout = [],
-        flameouts = 0,
-        timeout = this.options.timeout,
-        pids = this.options.pids;
+    cluster.on('fork',       this._workerForked.bind(this));
+    cluster.on('online',     this._workerOnline.bind(this));
+    cluster.on('listening',  this._workerListening.bind(this));
+    cluster.on('disconnect', this._workerDisconnected.bind(this));
+    cluster.on('exit',       this._workerExited.bind(this));
+};
 
-    cluster.on('fork', function (worker) {
-        startupTimeout[worker.id] = setTimeout(function () {
-            console.error('Something is wrong with worker %d', worker.id);
-        }, timeout);
-    });
+ComboCluster.prototype._workerForked = function (worker) {
+    this.startupTimeout[worker.id] = setTimeout(function () {
+        console.error('Something is wrong with worker %d', worker.id);
+    }, this.options.timeout);
+};
 
-    cluster.on('listening', function (worker) {
-        console.error('Worker %d listening with pid %d', worker.id, worker.process.pid);
-        clearTimeout(startupTimeout[worker.id]);
+ComboCluster.prototype._workerOnline = function (worker) {
+    console.error('Worker %d online', worker.id);
+    clearTimeout(this.startupTimeout[worker.id]);
 
-        // this doesn't work in OS X, but whatever
-        worker.process.title = 'combohandler worker';
-        pidfiles.writePidFile(pids, 'worker' + worker.id, worker.process.pid);
-    });
+    worker.send({ cmd: 'listen', data: this.options });
+};
 
-    cluster.on('disconnect', function (worker) {
-        console.error('Worker %d disconnecting...', worker.id);
-        closingTimeout[worker.id] = setTimeout(function () {
-            worker.destroy();
-            console.error('Forcibly destroyed worker %d', worker.id);
-        }, timeout);
-    });
+ComboCluster.prototype._workerListening = function (worker) {
+    console.error('Worker %d listening with pid %d', worker.id, worker.process.pid);
+    clearTimeout(this.startupTimeout[worker.id]);
 
-    cluster.on('exit', function (worker, code, signal) {
-        clearTimeout(startupTimeout[worker.id]);
-        clearTimeout(closingTimeout[worker.id]);
+    // this doesn't work in OS X, but whatever
+    worker.process.title = 'combohandler worker';
+    pidfiles.writePidFile(this.options.pids, 'worker' + worker.id, worker.process.pid);
+};
 
-        if (worker.suicide) {
-            console.error('Worker %d exited cleanly.', worker.id);
-            pidfiles.removePidFile(pids, 'worker' + worker.id);
+ComboCluster.prototype._workerDisconnected = function (worker) {
+    console.error('Worker %d disconnecting...', worker.id);
+    this.closingTimeout[worker.id] = setTimeout(function () {
+        worker.destroy();
+        console.error('Forcibly destroyed worker %d', worker.id);
+    }, this.options.timeout);
+};
+
+ComboCluster.prototype._workerExited = function (worker, code, signal) {
+    clearTimeout(this.startupTimeout[worker.id]);
+    clearTimeout(this.closingTimeout[worker.id]);
+
+    if (worker.suicide) {
+        console.error('Worker %d exited cleanly.', worker.id);
+        pidfiles.removePidFileSync(this.options.pids, 'worker' + worker.id);
+    }
+    else {
+        if (signal) {
+            console.error('Worker %d received signal %s', worker.id, signal);
+            if (signal === 'SIGUSR2') {
+                console.error('Worker %d restarting, removing old pidfile', worker.id);
+                pidfiles.removePidFileSync(this.options.pids, 'worker' + worker.id);
+            }
         }
-        else {
-            if (signal) {
-                console.error('Worker %d received signal %s', worker.id, signal);
-                if (signal === 'SIGUSR2') {
-                    console.error('Worker %d restarting, removing old pidfile', worker.id);
-                    pidfiles.removePidFile(pids, 'worker' + worker.id);
-                }
-            }
 
-            if (code) {
-                console.error('Worker %d exited with code %d', worker.id, code);
-                if (++flameouts > 20) {
-                    console.error("Too many errors during startup, bailing!");
-                    pidfiles.removePidFileSync(pids, 'master');
-                    process.exit(1);
-                }
+        if (code) {
+            console.error('Worker %d exited with code %d', worker.id, code);
+            if (++this.flameouts > 20) {
+                console.error("Too many errors during startup, bailing!");
+                pidfiles.removePidFileSync(this.options.pids, 'master');
+                process.exit(1);
             }
+        }
 
-            console.error('Worker %d died, respawning!', worker.id);
-            cluster.fork();
-        }
-    });
+        console.error('Worker %d died, respawning!', worker.id);
+        cluster.fork();
+    }
 };
@@ -171,0 +182,0 @@

lib/cluster/pidfiles.js

@@ -49,5 +49,3 @@ /**
 
-    fs.writeFile(getPidFilePath(dir, name), pid.toString(), function (err) {
-        if (err) { throw err; }
-    });
+    fs.writeFileSync(getPidFilePath(dir, name), pid.toString());
 }
@@ -54,0 +52,0 @@

lib/cluster/worker.js

@@ -29,2 +29,6 @@ /**
     process.title = 'combohandler worker';
+
+    // aid precise detaching during _destroy
+    this._boundDispatch = this.dispatch.bind(this);
+    process.on('message', this._boundDispatch);
 }
@@ -36,3 +40,20 @@
 
+ComboWorker.prototype.dispatch = function dispatch(msg) {
+    if (!msg || !msg.hasOwnProperty('cmd')) {
+        throw new Error("Message must have command");
+    }
+    var cmd = msg.cmd;
+    if (cmd === 'listen') {
+        if (msg.data) {
+            this.init(msg.data);
+        }
+        this.listen();
+    } else {
+        throw new Error("Message command invalid");
+    }
+};
+
 ComboWorker.prototype._destroy = function (cb) {
+    process.removeListener('message', this._boundDispatch);
+
     if (this._server) {
@@ -48,7 +69,12 @@ this._server.close(cb);
     this._server = app.listen(this.options.port);
+    this._server.once('listening', this._listening.bind(this));
 };
 
+ComboWorker.prototype._listening = function () {
+    this.emit('listening');
+};
+
 if (cluster.isWorker) {
     /*jshint newcap: false */
-    ComboWorker(require('../args').parse()).listen();
+    ComboWorker();
 }

package.json

 {
     "name"       : "combohandler",
     "description": "Simple Yahoo!-style combo handler.",
-    "version"    : "0.3.0",
+    "version"    : "0.3.1",
     "keywords"   : [
@@ -6,0 +6,0 @@ "combo", "combohandler", "combohandle", "combine", "cdn", "css", "yui"

README.md

@@ -29,7 +29,11 @@ Combo Handler
 
-    npm install combohandler
+```bash
+npm install combohandler
+```
 
 Or just clone the [GitHub repo](https://github.com/rgrove/combohandler):
 
-    git clone git://github.com/rgrove/combohandler.git
+```bash
+git clone git://github.com/rgrove/combohandler.git
+```
 
@@ -69,9 +73,13 @@
 
-    http://example.com/<route>?<path>[&path][...]
+```text
+http://example.com/<route>?<path>[&path][...]
+```
 
 For example:
 
-    http://example.com/foo?file1.js
-    http://example.com/foo?file1.js&file2.js
-    http://example.com/foo?file1.js&file2.js&subdir/file3.js
+```text
+http://example.com/foo?file1.js
+http://example.com/foo?file1.js&file2.js
+http://example.com/foo?file1.js&file2.js&subdir/file3.js
+```
 
@@ -205,3 +213,3 @@ Attempts to traverse above the `rootPath` or to request a file that doesn't
 
-```txt
+```text
 Usage: combohandler [options]
@@ -238,4 +246,6 @@
 
-    npm -g config set combohandler:port 2702
-    npm -g config set combohandler:server /path/to/server.js
+```bash
+npm -g config set combohandler:port 2702
+npm -g config set combohandler:server /path/to/server.js
+```
 
@@ -345,5 +355,7 @@ Unlike the `--server` option, a path specified in this manner *must* be absolute.
 
+```text
     http://example.com/combo/yui/3.9.1?yui/yui-min.js&yui-throttle/yui-throttle-min.js
     // vs
     http://example.com/combo/yui?3.9.1/build/yui/yui-min.js&3.9.1/build/yui-throttle/yui-throttle-min.js
+```
 
@@ -350,0 +362,0 @@ If the built-in `dynamicPath` middleware is used manually, it _must_ be inserted *before* the default `combine` middleware.

test/server.js

@@ -502,2 +502,6 @@ /*global describe, before, after, it */
             combo.respond);
+
+            app.get('/route-only/:version/lib',
+                combo.combine({ rootPath: __dirname + '/fixtures/root' }),
+            combo.respond);
         });
@@ -571,2 +575,13 @@
 
+        it("should work when param only found in route, not rootPath", function (done) {
+            request(BASE_URL + '/route-only/deadbeef/lib?js/a.js&js/b.js', function (err, res, body) {
+                assert.ifError(err);
+                res.should.have.status(200);
+                res.should.have.header('content-type', 'application/javascript; charset=utf-8');
+                res.should.have.header('last-modified');
+                body.should.equal('a();\n\nb();\n');
+                done();
+            });
+        });
+
         it("should error when param does not correspond to existing path", function (done) {
@@ -573,0 +588,0 @@ request(BASE_URL + '/dynamic/deadbeef?a.js', function (err, res, body) {

test/test-master.js

@@ -5,2 +5,3 @@ /*global describe, before, after, beforeEach, afterEach, it */
 var rimraf = require('rimraf');
+var mkdirp = require('mkdirp');
 
@@ -11,2 +12,3 @@ var ComboBase = require('../lib/cluster/base');
 describe("cluster master", function () {
+    var cluster = require('cluster');
     var PIDS_DIR = 'test/fixtures/pids';
@@ -45,2 +47,14 @@
         });
+
+        it("should setup instance properties", function () {
+            var instance = new ComboMaster();
+
+            instance.should.have.property('startupTimeout');
+            instance.should.have.property('closingTimeout');
+            instance.should.have.property('flameouts');
+
+            instance.startupTimeout.should.eql([]);
+            instance.closingTimeout.should.eql([]);
+            instance.flameouts.should.equal(0);
+        });
     });
@@ -52,6 +66,9 @@
 
-            instance._bindProcess();
+            instance._attachEvents();
+
+            hasAttachedClusterEvents();
             hasAttachedSignalEvents();
 
             instance.destroy(function () {
+                hasDetachedClusterEvents();
                 hasDetachedSignalEvents();
@@ -111,2 +128,9 @@
         });
+
+        it("should attach cluster events", function (done) {
+            master.emit('start', function () {
+                hasAttachedClusterEvents();
+                done();
+            });
+        });
     });
@@ -172,2 +196,108 @@
 
+    describe("handling cluster events", function () {
+        // ensure pids dir exists, no master pids created in these tests
+        before(function (done) {
+            mkdirp(PIDS_DIR, done);
+        });
+        after(cleanPidsDir);
+
+        var mockWorkerIds = 0;
+        function MockWorker() {
+            var id = mockWorkerIds++;
+            this.id = id;
+            this.process = {
+                pid: 1e6 + id
+            };
+        }
+        MockWorker.prototype.send = function (payload) {
+            this._sent = payload;
+        };
+        MockWorker.prototype.destroy = function () {
+            this._destroyed = true;
+        };
+
+        it("should set startupTimeout when worker forked", function () {
+            var instance = new ComboMaster({ timeout: 10 });
+            var worker = new MockWorker();
+
+            instance.startupTimeout.should.be.empty;
+            instance._workerForked(worker);
+            instance.startupTimeout.should.have.length(1);
+        });
+
+        it("should clear startupTimeout when worker online");
+
+        it("should send 'listen' command when worker online", function () {
+            var instance = new ComboMaster();
+            var worker = new MockWorker();
+
+            instance._workerOnline(worker);
+
+            worker._sent.should.eql({
+                cmd: 'listen',
+                data: instance.options
+            });
+        });
+
+        it("should write worker pidfile after worker listening", function () {
+            var instance = new ComboMaster({ pids: PIDS_DIR });
+            var worker = new MockWorker();
+
+            instance._workerListening(worker);
+
+            worker.process.should.have.property('title', 'combohandler worker');
+
+            fs.existsSync(path.join(instance.options.pids, 'worker' + worker.id + '.pid'));
+        });
+
+        it("should set closingTimeout when worker disconnected", function () {
+            var instance = new ComboMaster({ timeout: 10 });
+            var worker = new MockWorker();
+
+            instance.closingTimeout.should.be.empty;
+            instance._workerDisconnected(worker);
+            // timeouts aren't pushed onto the stack, they are assigned at the id's index
+            instance.closingTimeout.should.have.length(worker.id + 1);
+        });
+
+        it("should destroy() worker when closingTimeout expires", function (done) {
+            this.timeout(100);
+
+            var instance = new ComboMaster({ timeout: 10 });
+            var worker = new MockWorker();
+
+            instance._workerDisconnected(worker);
+
+            setTimeout(function () {
+                worker.should.have.property('_destroyed', true);
+                done();
+            }, 25);
+        });
+
+        it("should clear startupTimeout when worker exited");
+
+        it("should clear closingTimeout when worker exited");
+
+        it("should not fork a new worker when worker suicides");
+
+        it("should remove worker pidfile when worker suicides", function () {
+            this.timeout(100);
+
+            var instance = new ComboMaster({ pids: PIDS_DIR });
+            var worker = new MockWorker();
+
+            instance._workerListening(worker);
+
+            worker.suicide = true;
+
+            instance._workerExited(worker);
+
+            fs.readdirSync(instance.options.pids).should.not.include('worker' + worker.id + '.pid');
+        });
+
+        it("should remove worker pidfile when worker is reloaded");
+
+        it("should exit when flameouts threshhold exceeded");
+    });
+
     describe("on 'listen'", function () {
@@ -196,2 +326,18 @@ // disconnect called from #destroy() cleans the pids
 
+    function hasAttachedClusterEvents() {
+        cluster.listeners('fork'        ).should.have.length(1);
+        cluster.listeners('online'      ).should.have.length(1);
+        cluster.listeners('listening'   ).should.have.length(1);
+        cluster.listeners('disconnect'  ).should.have.length(1);
+        cluster.listeners('exit'        ).should.have.length(1);
+    }
+
+    function hasDetachedClusterEvents() {
+        cluster.listeners('fork'        ).should.be.empty;
+        cluster.listeners('online'      ).should.be.empty;
+        cluster.listeners('listening'   ).should.be.empty;
+        cluster.listeners('disconnect'  ).should.be.empty;
+        cluster.listeners('exit'        ).should.be.empty;
+    }
+
     function hasAttachedSignalEvents() {
@@ -198,0 +344,0 @@ process.listeners('SIGINT' ).should.have.length(1);

test/test-worker.js

@@ -28,2 +28,14 @@ /*global describe, it */
         });
+
+        it("should bind dispatcher to process", function () {
+            var instance = new ComboWorker();
+            var msgListeners = process.listeners('message').slice();
+            var dispatcherIndex = msgListeners.indexOf(instance._boundDispatch);
+
+            instance.should.have.property('_boundDispatch');
+            instance._boundDispatch.should.be.a('function');
+
+            // callback should be the last in the stack
+            dispatcherIndex.should.equal(msgListeners.length - 1);
+        });
     });
@@ -43,16 +55,91 @@
         });
+
+        it("should remove 'message' listener", function (done) {
+            var instance = new ComboWorker();
+
+            instance.destroy(function () {
+                var msgListeners = process.listeners('message').slice();
+                var dispatcherIndex = msgListeners.indexOf(instance._boundDispatch);
+
+                dispatcherIndex.should.equal(-1);
+
+                done();
+            });
+        });
     });
 
+    describe("on 'message'", function () {
+        it("should error when message missing", function (done) {
+            /*jshint immed:false */
+            var worker = new ComboWorker();
+
+            (function () {
+                worker.dispatch();
+            }).should.throwError("Message must have command");
+
+            worker.destroy(done);
+        });
+
+        it("should error when message command missing", function (done) {
+            /*jshint immed:false */
+            var worker = new ComboWorker();
+
+            (function () {
+                worker.dispatch({ data: { foo: "foo" } });
+            }).should.throwError("Message must have command");
+
+            worker.destroy(done);
+        });
+
+        it("should dispatch only matching commands", function (done) {
+            /*jshint immed:false */
+            var worker = new ComboWorker();
+
+            (function () {
+                worker.dispatch({ cmd: "poopypants" });
+            }).should.throwError("Message command invalid");
+
+            worker.destroy(done);
+        });
+
+        it("should dispatch 'listen' without data", function (done) {
+            var worker = new ComboWorker();
+
+            worker.listen = function () {
+                worker.destroy(done);
+            };
+
+            worker.dispatch({ cmd: "listen" });
+        });
+
+        it("should dispatch 'listen' with data", function (done) {
+            var worker = new ComboWorker();
+
+            var json = {
+                "cmd": "listen",
+                "data": { "foo": "foo" }
+            };
+
+            worker.listen = function () {
+                worker.options.should.have.property('foo');
+                worker.options.foo.should.equal('foo');
+                worker.destroy(done);
+            };
+
+            worker.dispatch(json);
+        });
+    });
+
     describe("on 'listen'", function () {
-        it("should create server", function (done) {
+        it("should create server and emit 'listening'", function (done) {
             var instance = new ComboWorker();
 
-            instance.listen();
-
-            process.nextTick(function () {
+            instance.once('listening', function () {
                 instance.should.have.property('_server');
                 instance.destroy(done);
             });
+
+            instance.listen();
         });
     });
 });

No alert changes

Improved metrics

Total package byte prevSize

114404

increased by9.7%

Lines of code

2305

increased by10.29%

Number of lines in readme file

401

increased by3.08%

No dependency changes