Response 'finish' sometimes called before stream completely written

[geek]

This is a difficult issue to reproduce, but I have narrowed it down to the following reproducing code:

var Assert = require('assert');
var Fs = require('fs');
var Http = require('http');
var Stream = require('stream');


var chunkTimes = 5;
var readTimes = 0;
var filePath = __dirname + '/biggie.js';
var responseJs = Fs.readFileSync(filePath).toString();

var expectedBody = '';
for (var i = 0, il = chunkTimes; i < il; ++i) {
    expectedBody += responseJs;
}

var streamServer = Http.createServer(function (req, res) {

    var fileStream = new Stream.Readable();
    fileStream._read = function (n) {

        if (readTimes++ === chunkTimes) {
            fileStream.push(null);
        }
        else {
            fileStream.push(responseJs);
        }
    };

    res.once('finish', function () {

        req.destroy();
    });

    fileStream.pipe(res);
}).listen(0);


streamServer.once('listening', function () {

    Http.get('http://127.0.0.1:' + streamServer.address().port, function (res) {

        var receivedFile = '';
        res.on('readable', function () {

            receivedFile += res.read().toString();
        });

        res.once('end', function () {

            Assert(receivedFile === expectedBody, 'Not equal results');
            process.exit();
        });
    }).on('error', function (err) {

        console.log(err);
    });
});

The key is to have a decent sized biggie.js file to try sending, mine needed to be over 2mb.

When you run the above code you should see the exception, if not try increasing the size of biggie.js.

[tjfontaine]

I believe this is a dup of #5688 can you apply 3c7945b and see if it solves it for you?

[geek]

Actually, no this didn't fix the issue.

[geek]

if instead of calling req.destroy() req.socket.destroySoon is called it works.

[geek]

I tried upgrading to node 0.10.12 and this is still an issue.

[isaacs]

Confirmed. This is definitely a bug, and not a dupe.

[isaacs]

To clarify: 'finish' should not happen before the last written chunk is flushed.

[isaacs]

See #5739 (comment)

Bottom line: We can't make this guarantee for HTTP streams. When 'finish' emits, it means that it's been sent to the socket, but NOT that it's necessarily made it to the client successfully.

I think that this sucks. But, if we try to make this guarantee, it'll fail for HTTPS streams anyway, and there's no way to fix that prior to 0.12.