All pages return 404 when using Express error handling

[byronh]

After seeing that the 404 and 500 modules are being deprecated, I tried to follow the suggested method of using Express's normal error handling.

The Express FAQ says the following:

All you need to do is add a middleware function at the very bottom of the stack (below all other functions) to handle a 404 response:

app.use(function(req, res, next) {
  res.status(404).send('Sorry cant find that!');
});

As expected, creating a Hello World app with just Express properly shows a 404 message for nonexistent routes and shows the working page at localhost:3000/working:

var app = require('express')();

app.use(require('kraken-js')());

app.get('/working', function(req, res) {
    res.send('Working page');
});

app.use(function(req, res, next) {
    res.status(404).send('404 page');
});

app.listen(3000);

However, creating a fresh Kraken project doing the same thing results in all routes returning 404s even if there are valid routes defined in the controllers folder:

var express = require('express');
var kraken = require('kraken-js');


var options, app;

/*
 * Create and configure application. Also exports application instance for use by tests.
 * See https://github.com/krakenjs/kraken-js#options for additional configuration options.
 */
options = {
    onconfig: function (config, next) {
        /*
         * Add any additional config setup or overrides here. `config` is an initialized
         * `confit` (https://github.com/krakenjs/confit/) configuration object.
         */
        next(null, config);
    }
};

app = module.exports = express();
app.use(kraken(options));
app.on('start', function () {
    console.log('Application ready to serve requests.');
    console.log('Environment: %s', app.kraken.get('env:env'));
});
app.use(function(req, res, next) {
    res.status(404).send('404 page');
});

Attempting to add the error handler to onconfig also results in all routes returning 404:

options = {
    onconfig: function (config, next) {
        /*
         * Add any additional config setup or overrides here. `config` is an initialized
         * `confit` (https://github.com/krakenjs/confit/) configuration object.
         */
        app.use(function(req, res, next) {
            res.status(404).send('404 page');
        });
        next(null, config);
    }
};

Is the only alternative to have duplicate error handlers in every single controller?

I also suggest that the proper method of error handling be documented in the readme, as it is currently very difficult to find up-to-date information on error handling in Kraken (especially for 500 errors).

[jasisk]

The problem here is in the way you're registering your middleware. In a kraken app, you register your middleware in a config file. If you app.use(something()) after you app.use(kraken()), your something() will actually occur first in the middleware chain (kraken does some async work).

In the case above, your 404 handler is handling everything, having been registered before the middleware configured by kraken. If you move it into the config and let kraken register it, all will be resolved.

In a kraken app, your app's entry point (typically index.js) should be almost completely bare. In fact, one of the main reasons (one of the few!) for kraken is to drive as much of the init process into a config. Middleware is registered from there. One of those middleware, by default, is express-enrouten which means routes can be added in a routes file or folder. And so on.

For example, the below is an application with a working route at /works, a 404 handler, and an unknown error handler. The working route is defined in routes.js (as enabled by default by kraken), the 404 handler is in lib/404.js and the error handler is in lib/error.js. Instead of writing a series of app.use statements, we register the 404 and error handlers via the config file (config/config.json). We set their priorities to 130 and 140 respectively, to ensure they're registered in the right order. The last middleware registered by kraken is 120, which is the default router. So if we want the 404 to be the last regular middleware, we set a priority of 121 or greater (like in this example which uses 130). Since we want the error handler middleware after the last of the regular middleware, we give it an even higher number of 140 (a number arbitrarily higher than the priority of the 404 handler).

// ========== index.js ==========
var kraken = require('kraken-js');
var express = require('express');

var app = express();

app.use(kraken());

app.listen(8000, function () {
  console.log('listening ...');
});

// ========== routes.js ==========
module.exports = function (router) {
  router.get('/works', function (req, res) {
    res.send('this works');
  });
};

// ========== lib/404.js ==========
module.exports = function () {
  return function (req, res, next) {
    res.status(404).send('uh oh');
  };
};

// ========== lib/error.js ==========
module.exports = function () {
  return function (err, req, res, next) {
    res.status(500).send('unknown error');
  };
};

// ========== config/config.json ==========
{
  "middleware": {
    "notFound": {
      "priority": 130,
      "module": "path:./lib/404"
    },
    "errorResponse": {
      "priority": 140,
      "module": "path:./lib/error"
    }  
  }  
}

[Knighton910]

👍 nicely answered @jasisk

[byronh]

Good explanation, thanks! I recommend describing this a bit more explicitly in the readme because Kraken handles middleware entirely different than Express, so pointing users to the Express docs could be a bit confusing.

[Knighton910]

cc @jasisk

close issue please 😄