Permalink
Browse files

change res.sendfile() to use send() module

  • Loading branch information...
1 parent 7a5041b commit ab61837885f8698f12105acbf0b41bb496e623c5 @tj tj committed Jul 13, 2012
View
@@ -8,24 +8,29 @@ var express = require('../../lib/express')
var app = module.exports = express();
-app.use(express.bodyParser());
-app.use(express.cookieParser('shhhh, very secret'));
-app.use(express.session());
+// config
app.set('view engine', 'ejs');
app.set('views', __dirname + '/views');
+// middleware
+
+app.use(express.bodyParser());
+app.use(express.cookieParser('shhhh, very secret'));
+app.use(express.session());
+
// Session-persisted message middleware
-app.locals.use(function(req,res){
+app.use(function(req, res, next){
var err = req.session.error
, msg = req.session.success;
delete req.session.error;
delete req.session.success;
res.locals.message = '';
if (err) res.locals.message = '<p class="msg error">' + err + '</p>';
if (msg) res.locals.message = '<p class="msg success">' + msg + '</p>';
-})
+ next();
+});
// dummy database
@@ -45,14 +45,6 @@ app.use(function(req, res, next){
next();
});
-// if you wanted to _always_ expose
-// the user you might do something like this:
-/*
-app.locals.use(function(req, res){
- if (req.user) res.locals.expose.user = req.user;
-})
-*/
-
app.get('/', function(req, res){
res.redirect('/user');
});
View
@@ -25,8 +25,24 @@ app.response.message = function(msg){
return this;
};
+// log
+if (!module.parent) app.use(express.logger('dev'));
+
+// serve static files
+app.use(express.static(__dirname + '/public'));
+
+// session support
+app.use(express.cookieParser('some secret here'));
+app.use(express.session());
+
+// parse request bodies (req.body)
+app.use(express.bodyParser());
+
+// support _method (PUT in forms etc)
+app.use(express.methodOverride());
+
// expose the "messages" local variable when views are rendered
-app.locals.use(function(req, res){
+app.use(function(req, res, next){
var msgs = req.session.messages || [];
// expose "messages" local variable
@@ -45,24 +61,9 @@ app.locals.use(function(req, res){
// empty or "flush" the messages so they
// don't build up
req.session.messages = [];
+ next();
});
-// log
-if (!module.parent) app.use(express.logger('dev'));
-
-// serve static files
-app.use(express.static(__dirname + '/public'));
-
-// session support
-app.use(express.cookieParser('some secret here'));
-app.use(express.session());
-
-// parse request bodies (req.body)
-app.use(express.bodyParser());
-
-// support _method (PUT in forms etc)
-app.use(express.methodOverride());
-
// load controllers
require('./lib/boot')(app, { verbose: !module.parent });
@@ -88,7 +88,6 @@ function count2(req, res, next) {
function users2(req, res, next) {
User.all(function(err, users){
if (err) return next(err);
- // this would not be ideal for *this*
res.locals.users = users.filter(ferrets);
next();
})
@@ -103,28 +102,6 @@ app.get('/middleware-locals', count2, users2, function(req, res, next){
});
-
-
-// let's assume we wanted to load the users
-// and count for every res.render() call, we
-// could use app.locals.use() for this. These
-// are callbacks which run in parallel ONLY
-// when res.render() is invoked. If no views
-// are rendered, there is no overhead.
-
-// This may be ideal if you want to load auxiliary
-// user information, but only for templates. Note
-// that (req, res) are available to you, so you may
-// access req.session.user etc.
-
-// Keep in mind these execute in *parallel*, so these
-// callbacks should not depend on each other, this
-// also makes them slightly more efficient than
-// using middleware which execute sequentially
-
-app.locals.use(count2);
-app.locals.use(users2);
-
app.get('/locals', function(req, res){
res.render('user', { title: 'Users' });
});
View
@@ -26,7 +26,7 @@ exports.init = function(app){
req.__proto__ = app.request;
res.__proto__ = app.response;
- res.locals = utils.locals(res);
+ res.locals = res.locals || utils.locals(res);
next();
}
View
@@ -12,6 +12,7 @@ var fs = require('fs')
, statusCodes = http.STATUS_CODES
, send = connect.static.send
, cookie = require('cookie')
+ , send = require('send')
, mime = connect.mime
, basename = path.basename
, extname = path.extname
@@ -171,38 +172,52 @@ res.sendfile = function(path, options, fn){
var self = this
, req = self.req
, next = this.req.next
- , options = options || {};
+ , options = options || {}
+ , done;
// support function as second arg
if ('function' == typeof options) {
fn = options;
options = {};
}
- // callback
- options.callback = function(err){
- if (err) {
- // cast ENOENT
- if ('ENOENT' == err.code) err = utils.error(404);
+ // socket errors
+ req.socket.on('error', error);
- // ditch content-disposition to prevent funky responses
- if (!self.headerSent) self.removeHeader('Content-Disposition');
+ // errors
+ function error(err) {
+ if (done) return;
+ done = true;
- // woot! callback available
- if (fn) return fn(err);
+ // clean up
+ req.socket.removeListener('error', error);
+ if (!self.headerSent) self.removeHeader('Content-Disposition');
- // lost in limbo if there's no callback
- if (self.headerSent) return;
+ // callback available
+ if (fn) return fn(err);
- return req.next(err);
- }
+ // list in limbo if there's no callback
+ if (self.headerSent) return;
- fn && fn();
- };
+ // delegate
+ next(err);
+ }
+
+ // streaming
+ function stream() {
+ if (done) return;
+ req.socket.removeListener('error', error);
+ if (fn) self.on('finish', fn);
+ }
// transfer
- options.path = encodeURIComponent(path);
- send(this.req, this, next, options);
+ var file = send(req, path);
+ if (options.root) file.root(options.root);
+ file.maxage(options.maxAge || 0);
+ file.on('error', error);
+ file.on('directory', next);
+ file.on('stream', stream);
+ file.pipe(this);
};
/**
View
@@ -18,6 +18,7 @@
"cookie": "0.0.3",
"fresh": "0.1.0",
"methods": "0.0.1",
+ "send": "0.0.2",
"debug": "*"
},
"devDependencies": {
View
@@ -17,9 +17,28 @@ describe('res', function(){
request(app)
.get('/')
- .end(function(res){
- done();
- })
+ .expect(200, done);
})
})
+
+ it('should work when mounted', function(done){
+ var app = express();
+ var blog = express();
+
+ app.use(blog);
+
+ blog.use(function(req, res, next){
+ res.locals.foo = 'bar';
+ next();
+ });
+
+ app.use(function(req, res){
+ res.locals.foo.should.equal('bar');
+ res.end();
+ });
+
+ request(app)
+ .get('/')
+ .expect(200, done);
+ })
})
View
@@ -11,6 +11,7 @@ describe('res', function(){
app.use(function(req, res){
res.sendfile('test/fixtures/user.html', function(err){
assert(!err);
+ req.socket.listeners('error').should.have.length(1); // node's original handler
done();
});
});
@@ -40,17 +41,17 @@ describe('res', function(){
app.use(function(req, res){
res.sendfile('test/fixtures/nope.html', function(err){
- assert(!res.headerSent);
++calls;
+ assert(!res.headerSent);
res.send(err.message);
});
});
request(app)
.get('/')
.end(function(err, res){
- calls.should.equal(1);
- res.text.should.equal('Not Found');
+ assert(1 == calls, 'called too many times');
+ res.text.should.equal("ENOENT, stat 'test/fixtures/nope.html'");
res.statusCode.should.equal(200);
done();
});
@@ -87,6 +88,25 @@ describe('res', function(){
.expect('Forbidden')
.expect(200, done);
})
+
+ it('should invoke the callback on socket error', function(done){
+ var app = express()
+ , calls = 0;
+
+ app.use(function(req, res){
+ res.sendfile('test/fixtures/user.html', function(err){
+ assert(!res.headerSent);
+ req.socket.listeners('error').should.have.length(1); // node's original handler
+ done();
+ });
+
+ req.socket.emit('error', new Error('broken!'));
+ });
+
+ request(app)
+ .get('/')
+ .end(function(){});
+ })
})
describe('.sendfile(path)', function(){

0 comments on commit ab61837

Please sign in to comment.