Add default 400 and 500 middleware to stack automatically

(instead of putting them in the explicit middleware order) We'll update the default `config/http.js` file to remove 404 and 500 from the order.
balderdashy · Nov 15, 2016 · a22e4e7 · a22e4e7
1 parent 176e085
commit a22e4e7
Show file tree

Hide file tree

Showing 5 changed files with 400 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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('<html>'));
+            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('<html>'));
+            assert(body.match('no file here bruh!'));
+            return done();
+          }
+        );
+
+      });
+
+      after(function(done) {
+        sailsApp.lower(done);
+      });
+
+    });
+
+  });
+
+});