Express middleware calls multiple times

Question

I've checked other posts related to this topic and couldn't find the problem in my code.

const myMiddleware = (fn) => {
    return (req, res, next) => {
        var fullUrl = req.protocol + '://' + req.get('host') + req.url;
        console.log(fullUrl)
        next()
    }
}

const app = express()

app.use('/dist', express.static(__dirname + '/client/dist'))
app.use('/static', express.static(__dirname + '/client/static'))

app.use(bodyParser.urlencoded({ extended: false }))
app.use(cookieParserMiddleware())
app.use(passport.initialize())

const server = https.createServer(options, app)

app.get('/', myMiddleware((req, res) => {
    res.sendFile(__dirname + '/client/dist/index.html')
}))

app.all('*', (req, res) => {
    res.redirect('/')
})

server.listen(8080, function listening() {
    console.log('Listening on %d', server.address().port)
})

Without the myMiddleware on '/' path everything works as expected. With myMiddleware attached as app.get('/', myMiddleware((req, res) => { then myMiddleware is called multiple times without res.sendFile(__dirname + '/client/dist/index.html') being called.

EDIT: The bug below is fixed with jfriend00's solution. The middleware getting called multiple times still exists. The reason is favicon and some other assets are not being captured by app.use('/static', express.static(__dirname + '/client/static')) line. Fixing path solved the first bug as well

On top of that problem, I've tried removing the part below but then the app doesn't work at all. My guess is there are 2 bugs here.

app.all('*', (req, res) => {
    res.redirect('/')
})

I have posted an image on how it looks when removing app.all('*'..)

Your middleware is not written correctly. I don't know what you're trying to do with it, but the callback you pass to it is never used. All it ever does is call `next()`. — jfriend00, Feb 09 '19 at 21:31
@jfriend00 Normally there is code written inside but to figure out what's wrong I have simplified it for testing. You can assume it has console.log() or some other logic inside — Thellimist, Feb 09 '19 at 21:36
Do you realize that `app.get('/', myMiddleware((req, res) => {...});` never does anything? All it does is call `next()`. That's my point. I don't know what you intended it to do, but it serves no purpose in your code example. I can't tell what your question really is so I don't know if this is related to the question or not. So, all your code does is redirect to `/` which redirects to `/` which redirects to `/` and so on. It never does anything else, but an infinite loop until the browser probably detects too many redirects. — jfriend00, Feb 09 '19 at 21:43
**Imagine** it does something. I edited the code to print url just for your sake. Without the middleware there is no redirection loop and the app works. WIth **any** middleware there is redirection loop. — Thellimist, Feb 09 '19 at 21:48
You need a route that does SOMETHING other than redirect. You don't show one. So, all your server does is redirect over and over. If you can't get that or show us more of your real code, then I'll move on. — jfriend00, Feb 09 '19 at 21:55
You are talking about `app.all('*')` not `'/'`. `'*'` does something which redirects all other links to home. It's a single page app. The line enforces this behavior. I didn't get why the posted error in the image happens when removing `app.all('*')` which might be another issue on top of the redirect loop — Thellimist, Feb 09 '19 at 21:59
From the code you show `res.sendFile(__dirname + '/client/dist/index.html')` is NEVER called. So, every single request that isn't satisfied by an `express.static()` route ends up in your `app.all('*', ...)` and redirects which will then just redirect and so on. — jfriend00, Feb 09 '19 at 22:01
`res.sendFile(__dirname + '/client/dist/index.html')` is called when I delete the middleware. Could you explain why there is this behaviour. https://stackoverflow.com/a/16637468/4335588 — Thellimist, Feb 09 '19 at 22:02
"delete the middleware" isn't very specific. You have at least 6 lines of middleware. I give up. Hopefully someone else will come along and try to understand the question. — jfriend00, Feb 09 '19 at 22:11
Yeah, that's because `myMiddleware` NEVER calls the `fn` you pass it. That's the point I made in my very first comment. There is no `fn()` anywhere to ever call that in your implementation. — jfriend00, Feb 09 '19 at 22:18

jfriend00 · Accepted Answer · 2019-02-09T22:27:12.953

2

I'm going to make a guess here.

Change this:

app.get('/', myMiddleware((req, res) => {
    res.sendFile(__dirname + '/client/dist/index.html')
}));

to this:

app.get('/', myMiddleware(), (req, res) => {
    res.sendFile(__dirname + '/client/dist/index.html')
}));

So that your res.sendFile() actually gets called after the myMiddleware calls next().

Then, remove the fn argument so you have this (doesn't change execution, but removes misleading and unused argument):

const myMiddleware = () => {
    return (req, res, next) => {
        var fullUrl = req.protocol + '://' + req.get('host') + req.url;
        console.log(fullUrl)
        next()
    }
}

edited Feb 09 '19 at 22:27

answered Feb 09 '19 at 22:21

jfriend00

580,699
78
809
825

I mentioned there were two bugs. (1) multiple calls to middleware (2) deleting `app.all('*', (req, res) => {` would cause errors. With your changes applied we've This fixed bug #2, but, calling localhost:8080 would call `myMiddleware` 3 times – Thellimist Feb 09 '19 at 22:38
1

@Thellimist - That's not surprising. One call to your server will be for the favicon. Do you have any `` ` – jfriend00 Feb 09 '19 at 22:40
I have `static/css/lobby.css` and `static/css/style.css` and favicon. Is it normal for them to call or is there a better way? – Thellimist Feb 09 '19 at 22:45
@Thellimist - That's how the browser gets those files. It makes a request back to your server for each one. That's how web pages work. – jfriend00 Feb 09 '19 at 22:58
they are calling the `static/` path so I assumed the `express.static` middleware would catch that request and not pass it all the way to `myMiddleware`. Am I missing something? – Thellimist Feb 09 '19 at 23:03
@Thellimist - If those requests are getting past your `express.static()` middleware, then that middleware isn't matching them properly. Either they aren't configured correctly or the URLs you're asking for aren't right. We can't do anything here about that without knowing both the URLs that are being request and seeing exactly where the desired matching files are in your file system. It's a very common mistake to specific `express.static()` stuff incorrectly. – jfriend00 Feb 09 '19 at 23:12

Express middleware calls multiple times

1 Answers1