Reputation: 6462
I have this issue where inside a router file that returns a router object - middleware is getting called after a response is already made.
I thought middleware placed after a route will never get executed for that route, I especially thought that would be true if a response was already given.
Main question:
Middleware should stop being called after the response right?
Ex: When calling the endpoint "/login" and sending a response both middleware A and B are called.
Note: I have wrapped the router in a higher order function (for socket.io functionality but that is beside the point).
Code Example:
var express = require('express');
var router = express.Router();
var wrappedRouter = function(val) {
router.get('/login', function(req, res) {
console.log("Inside login route");
res.sendFile(path.resolve('login.html'))
}
router.use(function(req, res, next) {
console.log("Exectuing middleware A");
});
router.use(function(req, res, next) {
console.log("Exectuing middleware B");
});
router.get('/another-route', function(req, res) {
res.sendFile(path.resolve('login.html'))
}
return router;
}
//Prints:
// Inside login route
// Exectuing middleware A
// Exectuing middleware B
Upvotes: 2
Views: 3004
Reputation: 2319
Your assumption is absolutely correct, if you do not call next, next middleware in chain will never get executed, The problem you are facing is because, as you have already guessed, due to static resources your page is requesting, like favicon or images or anything else (separate requests). To get concrete proof, you can log url while serving express requests and see what requests are made and what gets executed using req.url
as shown below
var express = require('express');
var router = express.Router();
var path = require('path');
var wrappedRouter = function(val) {
router.get('/login', function(req, res) {
console.log("Inside login route: " + req.url);
res.sendFile(path.resolve('login.html'))
});
router.use(function(req, res, next) {
console.log("Exectuing middleware A: " + req.url);
});
router.use(function(req, res, next) {
console.log("Exectuing middleware B: " + req.url);
});
router.get('/another-route', function(req, res) {
res.sendFile(path.resolve('login.html'))
});
return router;
}
module.exports = wrappedRouter({});
Notice req.url
in console.log("Inside login route: " + req.url);
this will append current req resource and we'll be able to identify whats called and why. As shown below
So now we know whats happening and why, now lets see how we can resolve it.
In express the order in which we register stuff to express app is very important. And I'd suggest registering your middleware separately (first) and then register your routes, this will increase the manageability of your super awesome code!
That being said, coming to the problem at hand, as sequence matters, what is happening in your case it, express tries each and every middleware in sequence and until it runs out of it. So, when server asks for 'favicon' (or some other static resource), it starts executing matching middleware in sequence. "Middleware A" handles the request and generates the output (keep in mind that the favicon is not served, actually nothing happens and the request will keep on handing until you call next or send something in response.
The solution to your problem is registering a static folder in the beginning of your app.js, before anything gets registered. This way when static resources are requested, they are served right away, and other requests which are not entertained by static resource middleware are passed on to your middleware.
It is not advisable to serve files are you are doing, as the app grows you may find it hard to manage, instead you should use view engines / static middleware to get what you are trying to achieve, as shown in below code.
Your app structure should be as follows, and it will resolve your issue.
var express = require('express');
var path = require('path');
var favicon = require('serve-favicon');
var logger = require('morgan');
var cookieParser = require('cookie-parser');
var bodyParser = require('body-parser');
//This will return wrapped routers
var routes = require('./routes/index');
var other = require('./routes/other');
var app = express();
// view engine setup
app.set('views', path.join(__dirname, 'views'));
app.set('view engine', 'jade');
// uncomment after placing your favicon in /public
//app.use(favicon(path.join(__dirname, 'public', 'favicon.ico')));
app.use(logger('dev'));
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: false }));
app.use(cookieParser());
//Just after registering all request parsing middleware, register static middleware
app.use(express.static(path.join(__dirname, 'public')));
app.use('/', routes);
app.use('/others', other);
// catch 404 and forward to error handler
app.use(function(req, res, next) {
var err = new Error('Not Found');
err.status = 404;
next(err);
});
// error handlers
// development error handler
// will print stacktrace
if (app.get('env') === 'development') {
app.use(function(err, req, res, next) {
res.status(err.status || 500);
res.render('error', {
message: err.message,
error: err
});
});
}
// production error handler
// no stacktraces leaked to user
app.use(function(err, req, res, next) {
res.status(err.status || 500);
res.render('error', {
message: err.message,
error: {}
});
});
module.exports = app;
If you are new to express and want to get started and see what works how, I have written a small blog about it, Part one is done, rest are in queue ;)
Get started with Express and build RESTful api In this part, I show how to scaffold an express app, above code from it. You might find it helpful.
Hope it helps! Please let me know if you need anything else!
Upvotes: 5
Reputation: 2241
Express is executing middleware in same order that you defined it and lifecycle does not end with res.send()
.
You can use them e.g. to postprocess even when request was already satisfied.
var wrappedRouter = function(val) {
router.use(function(req, res, next) {
request.startProcessingTime = +(new Date());
next();
});
router.get('/another-route', function(req, res) {
res.sendFile(path.resolve('login.html'))
}
(...)
router.use(function(req, res, next) {
req.endProcessingTime = +(new Date());
next();
});
router.use(function(req, res, next) {
console.log('Processing time:', req.endProcessingTime - req.startProcessingTime)
});
return router;
}
Upvotes: 1
Reputation: 48516
Another thing I should say, please make sure the next()
is called in the router.use()
as below.
router.use(function(req, res, next) {
console.log("Exectuing middleware A");
next();
});
router.use(function(req, res, next) {
console.log("Exectuing middleware B");
next();
});
router.get('/login', function(req, res) {
console.log("Inside login route");
res.send('Hello World');
});
Which output
Exectuing middleware A
Exectuing middleware B
Inside login route
If the next()
does not invoked in the router.use()
router.use(function(req, res, next) {
console.log("Exectuing middleware A");
//next();
});
router.use(function(req, res, next) {
console.log("Exectuing middleware B");
//next();
});
router.get('/login', function(req, res) {
console.log("Inside login route");
res.send('Hello World');
});
Output:
Exectuing middleware A
And the response is pending in the browser for this request...
Upvotes: 1