Close write stream on connection error (FD leak on incomplete uploads)

[Khez]

We have seen a large number of open FD's when using express with multer.

After investing, we have found the issue to be aborted uploads.

It seems the upload FD is not being closed if the client aborts the request mid-way.

How To Reproduce

From the readme (adapted to a single field):

const express = require('express');
const multer  = require('multer');
const upload = multer({ dest: 'uploads/' });

var app = express();
var cpUpload = upload.fields([{
    name: 'avatar',
    maxCount: 1
}]);

app.post('/upload', cpUpload, function (req, res, next) {
    res.send('ok');
});

app.listen(8081, () => {
    console.log('Listening');
});

Using whichever client, upload a file and abort it during transit:

# make sure the file is somewhat large
# and don't forget to abort before getting the response
curl -vvv 'http://localhost:8081/upload' -F 'avatar=@file' --limit-rate 1M;

Finally using lsof on the node process, you should see a FD leaked to the uploads directory

Related(?)

• http.IncomingMessage omits events when aborted on client side  nodejs/node#7732
  • Really old bug, closed without being solved, seems like this is is the "core" issue
• Sporadic ERR_STREAM_PREMATURE_CLOSE fastify/fastify-static#116 (comment)
  • Similar reproduction steps, but on downloading files

[Khez]

I'm saddened to see this ignored. This was a very visible issue and with a clear solution.

We attempted a similar fix in production, but we still had some FD Leaks, uncertain why. It looked something like:

const express = require('express');
const multer  = require('multer');
const upload = multer({ dest: 'uploads/' });

var app = express();
var cpUpload = upload.fields([{
    name: 'avatar',
    maxCount: 1
}]);


app.post('/upload', function (req, res, next) {
    // Userland fix attempt for https://github.com/expressjs/multer/pull/971
    (req.connection || req.socket).on('error', (error) => {
        console.log('Connection Error Detected', error.message || error);
        req.emit('end');
    });

    cpUpload(req, res, function(err) {
        console.log(err || 'Upload Done');
        res.send('ok');
    });
});

app.listen(8081, () => {
    console.log('Listening');
});

Due to this PR gathering dust and the fact that we still had FD leaks, we ended up moving to formidable. Haven't had any leaks since then.