Clarity on if it's safe to call next() after send()

[kwasimensah]

Branching https://github.com/expressjs/serve-static/issues/136#issuecomment-661169863 since it's more general than express-static

The use case I'm trying to catch is when the client aborts. I found in my unit testing it's actually hard to get express/https.server to shutdown cleanly i.e. if the client aborts while the request is asynchronously talking to the database, https.server.close won't wait for that to finish/won't wait for any in progress async middleware to finished before the "close" callback is called.

I'm trying to add middleware at the beginning and end of each request to track what's still in progress independent of the connection being severed so I can delay fully closing if there's any middleware that might still use external resources. on-finished isn't what I'm looking for because if the client closes the connection in the middle of a request using async middleware "close" will be fired before all the middleware has finished.

I'm trying to find official expressjs documentation saying it's unsafe to process middleware after sending the response (as long as you don't try to edit anything about the request object). I just realized I got the idea from this non-official article I know saying "it works for me" is not a sign that it's a supported use case but it is working so far.

Should this be resolved by:

1. adding explicit wording in the documentation saying calling next() after end() is wrong and it should throw an error. I know I'm being pedantic but the docs currently say

If the current middleware function does not end the request-response cycle, it must call next() to pass control to the next middleware function.

without being specific about about what's considered the full request-response cycle.

2. making this a supported use case for people who don't want to block sending a response for server specific work tied to the middleware chain (and not specifically the connection) and adding tests to spot regressions.

3. adding a dummy middleware that just calls res.end() and acts like 2) but with a delay and using https://github.com/expressjs/serve-static/issues/136#issuecomment-661125380 for serve-static.

I'm a fan of 2. The only other option I see is adding checks after every promise in middleware to see if the request is active but that doesn't catch issues if the request is closed mid-await.

[kwasimensah]

To clarify, yes, I tried listening to "finish" and "close". Given:

async function myDatabaseMiddleWare(req, res, next) {
   await databaseQuery1();
   await databaseQuery2();
  next();
}

if server.close is called between databaseQuery1 and databaseQuery2, the "close" event will fire for req but the middleware will still try to call databaseQuery2. Express doesn't try to stop the rest of the middleware function.

This means server.close will return before its safe to actually tear down the database connection because it will still be used for databaseQuery2. I'm trying to catch this with the (super simplified) code below.

let requestsStillRunningMiddleware = 0;

// at the start of the middleware stack
app.use((req, res, next)=>{
  ++requestsStillRunningMiddleware;
  next();
})


// at the end of the middleware stack
app.use((req, res, next)=>{
  --requestsStillRunningMiddleware;
  if(requestsStillRunningMiddleware=== 0){
      // signal that it's safe to clean up resources used in middleware.
   }
})

But it requires all middleware to call next, including the one that actually calls send().

Yes, I am trying to implement a graceful shutdown system. There are some examples on the Express website: http://expressjs.com/en/advanced/healthcheck-graceful-shutdown.html. All of them keep track of connections between the client and the server. They do not keep track of connections between the server and backend services (postgres/redis/etc) except through explicit health checks. But this doesn't allow you to make sure no one is using those resources before you consider the server "closed" (I guess you could make the health checks return something other than just a 200 but then you're opening yourself up to race conditions).