diff --git a/lib/hooks/http/get-configured-http-middleware-fns.js b/lib/hooks/http/get-configured-http-middleware-fns.js index 21a4716b2a..a9b6ff2355 100644 --- a/lib/hooks/http/get-configured-http-middleware-fns.js +++ b/lib/hooks/http/get-configured-http-middleware-fns.js @@ -215,9 +215,6 @@ module.exports = function getBuiltInHttpMiddleware (expressRouterMiddleware, sai // 404 and 500 middleware should be attached at the very end // (after `router`, `www`, and `favicon`) - // - // * TODO: make these both implicit (we can just tack them on to the end of the - // * middleware stack automatically--it's confusing that they appear to be configurable) 404: function handleUnmatchedRequest(req, res, next) { // Explicitly ignore error arg to avoid inadvertently diff --git a/lib/hooks/http/index.js b/lib/hooks/http/index.js index 2715a3b8d6..3d51eaebf6 100644 --- a/lib/hooks/http/index.js +++ b/lib/hooks/http/index.js @@ -60,9 +60,7 @@ module.exports = function(sails) { '$custom', 'router', 'www', - 'favicon', - '404', - '500' + 'favicon' ], // Built-in HTTP middleware functions are injected after the express diff --git a/lib/hooks/http/initialize.js b/lib/hooks/http/initialize.js index 1677d06cf3..f563bb3bd0 100644 --- a/lib/hooks/http/initialize.js +++ b/lib/hooks/http/initialize.js @@ -233,6 +233,12 @@ module.exports = function(sails) { } }); + // Add the default 404 to the middleware order + postRouterMiddleware.push('404'); + + // Add the default 500 to the middleware order + postRouterMiddleware.push('500'); + // If a custom `loadMiddleware` function was configured, then call it to "use" // the configured middleware (instead of doing it automatically with the more // modern `sails.config.http.middleware.order` configuration). diff --git a/test/integration/middleware.404.test.js b/test/integration/middleware.404.test.js new file mode 100644 index 0000000000..2653566a46 --- /dev/null +++ b/test/integration/middleware.404.test.js @@ -0,0 +1,196 @@ +var _ = require('@sailshq/lodash'); +var request = require('request'); +var Sails = require('../../lib').Sails; +var assert = require('assert'); +var fs = require('fs-extra'); +var request = require('request'); +var appHelper = require('./helpers/appHelper'); +var path = require('path'); + +describe('middleware :: ', function() { + + describe('404 :: ', function() { + + var appName = 'testApp'; + var sailsApp; + + before(function(done) { + appHelper.build(function(err) { + if (err) {return done(err);} + fs.writeFileSync(path.resolve('..', appName, 'views', '404.ejs'), 'no file here bruh!'); + return done(); + }); + }); + + after(function() { + process.chdir('../'); + appHelper.teardown(); + }); + + describe('with no custom 404 handler installed', function() { + + before(function(done) { + appHelper.lift({ + hooks: { + pubsub: false + } + }, function(err, _sailsApp) { + if (err) { return done(err); } + sailsApp = _sailsApp; + return done(); + }); + }); + + it('the default 404 handler should respond to a request for an unbound URL', function(done) { + + request( + { + method: 'GET', + uri: 'http://localhost:1342/nothing', + headers: { + 'Accept': 'text/html' + } + }, + function(err, response, body) { + if (err) { return done(err); } + assert.equal(response.statusCode, 404); + assert(body.match('')); + assert(body.match('no file here bruh!')); + return done(); + } + ); + + }); + + + after(function(done) { + sailsApp.lower(done); + }); + + }); + + describe('with a custom 404 handler installed', function() { + + before(function(done) { + appHelper.lift({ + hooks: { + pubsub: false + }, + http: { + middleware: { + order: [ + 'startRequestTimer', + 'cookieParser', + 'session', + 'bodyParser', + 'handleBodyParserError', + 'compress', + 'methodOverride', + 'poweredBy', + '$custom', + 'router', + 'www', + 'favicon', + 'notfound' + ], + notfound: function (req, res) { + return res.send('custom nada bro'); + } + } + } + }, function(err, _sailsApp) { + if (err) { return done(err); } + sailsApp = _sailsApp; + return done(); + }); + }); + + it('the custom 404 handler should respond to a request for an unbound URL', function(done) { + + request( + { + method: 'GET', + uri: 'http://localhost:1342/nothing', + headers: { + 'Accept': 'text/html' + } + }, + function(err, response, body) { + if (err) { return done(err); } + assert.equal(response.statusCode, 200); + assert.equal(body, 'custom nada bro'); + return done(); + } + ); + + }); + + + after(function(done) { + sailsApp.lower(done); + }); + + }); + + describe('with 404 left out of a custom middleware order', function() { + + before(function(done) { + appHelper.lift({ + hooks: { + pubsub: false + }, + http: { + middleware: { + order: [ + 'startRequestTimer', + 'cookieParser', + 'session', + 'bodyParser', + 'handleBodyParserError', + 'compress', + 'methodOverride', + 'poweredBy', + '$custom', + 'router', + 'www', + 'favicon' + ] + } + } + }, function(err, _sailsApp) { + if (err) { return done(err); } + sailsApp = _sailsApp; + return done(); + }); + }); + + it('the default 404 handler should still respond to a request for an unbound URL', function(done) { + + request( + { + method: 'GET', + uri: 'http://localhost:1342/nothing', + headers: { + 'Accept': 'text/html' + } + }, + function(err, response, body) { + if (err) { return done(err); } + assert.equal(response.statusCode, 404); + assert(body.match('')); + assert(body.match('no file here bruh!')); + return done(); + } + ); + + }); + + after(function(done) { + sailsApp.lower(done); + }); + + }); + + }); + +}); diff --git a/test/integration/middleware.500.test.js b/test/integration/middleware.500.test.js new file mode 100644 index 0000000000..3b37c2d674 --- /dev/null +++ b/test/integration/middleware.500.test.js @@ -0,0 +1,197 @@ +var _ = require('@sailshq/lodash'); +var request = require('request'); +var Sails = require('../../lib').Sails; +var assert = require('assert'); +var fs = require('fs-extra'); +var request = require('request'); +var appHelper = require('./helpers/appHelper'); +var path = require('path'); + +describe('middleware :: ', function() { + + describe('500 :: ', function() { + + var appName = 'testApp'; + var sailsApp; + + before(function(done) { + appHelper.build(function(err) { + if (err) {return done(err);} + fs.writeFileSync(path.resolve('..', appName, 'views', '500.ejs'), 'bogus err bruh!'); + fs.writeFileSync(path.resolve('..', appName, 'config', 'routes.js'), 'module.exports.routes = { \'/err\': function (req, res) {throw new Error(\'errrr\');} };'); + return done(); + }); + }); + + after(function() { + process.chdir('../'); + appHelper.teardown(); + }); + + describe('with no custom 500 handler installed', function() { + + before(function(done) { + appHelper.lift({ + hooks: { + pubsub: false + } + }, function(err, _sailsApp) { + if (err) { return done(err); } + sailsApp = _sailsApp; + return done(); + }); + }); + + it('the default 500 handler should respond to a request that causes an error', function(done) { + + request( + { + method: 'GET', + uri: 'http://localhost:1342/err', + headers: { + 'Accept': 'text/html' + } + }, + function(err, response, body) { + if (err) { return done(err); } + assert.equal(response.statusCode, 500); + assert(body.match('')); + assert(body.match('bogus err bruh!')); + return done(); + } + ); + + }); + + + after(function(done) { + sailsApp.lower(done); + }); + + }); + + describe('with a custom 500 handler installed', function() { + + before(function(done) { + appHelper.lift({ + hooks: { + pubsub: false + }, + http: { + middleware: { + order: [ + 'startRequestTimer', + 'cookieParser', + 'session', + 'bodyParser', + 'handleBodyParserError', + 'compress', + 'methodOverride', + 'poweredBy', + '$custom', + 'router', + 'www', + 'favicon', + 'err' + ], + err: function (err, req, res, next) { + return res.send('custom err bro'); + } + } + } + }, function(err, _sailsApp) { + if (err) { return done(err); } + sailsApp = _sailsApp; + return done(); + }); + }); + + it('the custom 500 handler should respond to a request that causes an error', function(done) { + + request( + { + method: 'GET', + uri: 'http://localhost:1342/err', + headers: { + 'Accept': 'text/html' + } + }, + function(err, response, body) { + if (err) { return done(err); } + assert.equal(response.statusCode, 200); + assert.equal(body, 'custom err bro'); + return done(); + } + ); + + }); + + + after(function(done) { + sailsApp.lower(done); + }); + + }); + + describe('with 500 left out of a custom middleware order', function() { + + before(function(done) { + appHelper.lift({ + hooks: { + pubsub: false + }, + http: { + middleware: { + order: [ + 'startRequestTimer', + 'cookieParser', + 'session', + 'bodyParser', + 'handleBodyParserError', + 'compress', + 'methodOverride', + 'poweredBy', + '$custom', + 'router', + 'www', + 'favicon' + ] + } + } + }, function(err, _sailsApp) { + if (err) { return done(err); } + sailsApp = _sailsApp; + return done(); + }); + }); + + it('the default 500 handler should respond to a request that causes an error', function(done) { + + request( + { + method: 'GET', + uri: 'http://localhost:1342/err', + headers: { + 'Accept': 'text/html' + } + }, + function(err, response, body) { + if (err) { return done(err); } + assert.equal(response.statusCode, 500); + assert(body.match('')); + assert(body.match('bogus err bruh!')); + return done(); + } + ); + + }); + + after(function(done) { + sailsApp.lower(done); + }); + + }); + + }); + +});