Description

RoobinGood

opened on Aug 14, 2016

Issue body actions

• Version: v4.4.0
• Platform: Linux robin-master 3.19.0-15-generic #15-Ubuntu SMP Thu Apr 16 23:32:37 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: http

Long client requests ends successfully if data transfer closed for some reason, for example if server lost connectivity or suddenly fall.
For example I have one client Node.js app which make request to server Node.js app to receive a lot of chunks of data. Data transfer lasts for several time and server app falls before request completion.
I expect to get some error on client side to inform me that transfer was not successfully ended, but emits end event instead of error. So I have a situation when I receive only part of data but don't know about it and think that request successfully ended.

Example server send parts of JSON object for ~2 seconds. For test purpose I run following shell command:

timeout 1s node server.js & node client.js

Because data transfer lasts 2 seconds client app receive only part of data but ends with emit end handler without any error. May be problem on clientRequest part of Node.js cause the same trick with curl as client side return (18) transfer closed error.

Example of client app:

var http = require('http');

var makeRequest = function() {
    var req = http.request({
        host: '127.0.0.1',
        port: 8080,
        method: 'POST',
    }, function(res) {
        var data = '';
        res.on('data', function(chank) {
            data += chank;
        });

        res.on('end', function() {
            console.log('data', data);
            console.log('succesfull end');
        });

        res.on('error', function(err) {
            console.error(err);
        });
    });

    req.on('error', function(err) {
        console.error(err);
    });

    req.write('');
    req.end();
};

setTimeout(makeRequest, 100);

Example of server app

var http = require('http');

var server = http.createServer();
server.on('request', function (req, res) {

    res.write('{"result":[{"some": "object"}');

    // it must send data pieces for ~2 sec (20*100 ms = 2000 ms) 
    var times = 20;
    var delay = 100;

    var counter = 0;
    var timerId = setInterval(function() {
        res.write(',{"some": "object"}');

        counter++;
        if (counter >= times) {
            res.write(',{"some": "object"}');
            res.write(']}');
            res.end();
            clearInterval(timerId);
        }
    }, delay);
});

var port = 8080;
server.listen(port, function() {
    console.log('listen on 127.0.0.1:' + port);
});

Activity

1. RoobinGood commented on Aug 14, 2016

   

   RoobinGood

   on Aug 14, 2016

   Author

   More actions

   Also tested on v4.4.7 with the same result.
2. MylesBorins commented on Aug 14, 2016

   

   MylesBorins

   on Aug 14, 2016

   Contributor

   More actions

   /cc @bnoordhuis I seem to recall you had answered a similar question before
3. mscdex

   added

   httpIssues and PRs related to the http subsystem.Issues and PRs related to the http subsystem.

   on Aug 14, 2016
4. bnoordhuis

   added

   docIssues and PRs related to Node.js documentation.Issues and PRs related to Node.js documentation.

   testIssues and PRs related to Node.js core tests and test infrastructure.Issues and PRs related to Node.js core tests and test infrastructure.

   docs-requested

   on Aug 15, 2016
5. bnoordhuis commented on Aug 15, 2016

   

   bnoordhuis

   on Aug 15, 2016

   Member

   More actions

   The client sees an empty read when the server is killed. You can verify that with strace.

   curl expects a zero-sized HTTP chunk because it sends Connection: keep-alive. The node.js client on the other hand sends Connection: close, in which case it's proper to end the response by simply closing the connection.

   Node.js does not raise an error when keep-alive is enabled and the connection is closed before the final zero chunk but you can check if res.complete === true in your 'end' event listener.

   res.complete apparently was never documented and we don't seem to have test coverage for it either.

   It's been around for a long time though, it was added when support for trailing HTTP headers was implemented in node.js v0.3.0. Adding doc and test labels.
6. okv commented on Aug 15, 2016

   

   okv

   on Aug 15, 2016

   More actions

   Hi, there.

   I encountered same problem. I'll try res.complete flag, thanks for explanation.

   Node.js does not raise an error when keep-alive is enabled and the connection is closed before the final zero chunk

   Why Node.js does not raise an error in such situation? It looks reasonable.
7. RoobinGood commented on Aug 15, 2016

   

   RoobinGood

   on Aug 15, 2016

   Author

   More actions

   @bnoordhuis thanks for explanation, res.complete really works.
8. bnoordhuis commented on Aug 15, 2016

   

   bnoordhuis

   on Aug 15, 2016

   Member

   More actions

   Why Node.js does not raise an error in such situation? It looks reasonable.

   I'm not 100% sure but I think it's for compatibility. Not all HTTP endpoints follow the spec as closely as they should.

   You could argue, tongue in cheek, that closing the connection without sending the zero chunk is a performance optimization - saves a TCP round-trip!
9. jasnell commented on Aug 15, 2016

   

   jasnell

   on Aug 15, 2016

   Member

   More actions

   Yep.. there are some implementations that have been rather pathological about not properly terminating a chunked stream. While throwing would make sense in theory, it ends up being a bit problematic in practice.
10. okv commented on Aug 15, 2016

    

    okv

    on Aug 15, 2016

    More actions

    got it, thanks
11. sam-github

    added

    docIssues and PRs related to Node.js documentation.Issues and PRs related to Node.js documentation.

    and removed

    docIssues and PRs related to Node.js documentation.Issues and PRs related to Node.js documentation.

    docs-requested

    on Dec 1, 2016
12. Trott commented on Jul 15, 2017

    

    Trott

    on Jul 15, 2017

    Member

    More actions

    @nodejs/documentation @nodejs/testing
13. Trott commented on Jul 15, 2017

    

    Trott

    on Jul 15, 2017

    Member

    More actions

    @nodejs/http
14. refack commented on Jul 17, 2017

    

    refack

    on Jul 17, 2017

    Contributor

    More actions

    @jasnell does #14315 improve this situation?
15. Redocram commented on Nov 3, 2017

    

    Redocram

    on Nov 3, 2017

    More actions

    Hi everybody,
    I have the same problem with my code, or I think so.

    May I have a short example of the implementation of the if (res.complete === true) statement for a post request?

    Thanks all!
16. jasnell

    added a commit that references this issue on Oct 26, 2018

    doc: add documentation for http.IncomingMessage$complete

    f1a4ce8
17. jasnell

    closed this as completedin bd8c107on Oct 30, 2018
18. targos

    added a commit that references this issue on Nov 1, 2018

    doc: add documentation for http.IncomingMessage$complete

    24c6a02
19. MylesBorins

    added 2 commits that reference this issue on Nov 27, 2018

    doc: add documentation for http.IncomingMessage$complete

    3c46c03

    doc: add documentation for http.IncomingMessage$complete

    f0bbe9b
20. rvagg

    added 2 commits that reference this issue on Nov 28, 2018

    doc: add documentation for http.IncomingMessage$complete

    6d76f85

    doc: add documentation for http.IncomingMessage$complete

    893239b
21. MylesBorins

    added a commit that references this issue on Nov 29, 2018

    doc: add documentation for http.IncomingMessage$complete

    c68aab1
22. sjarmak

    added a commit that references this issue on Jul 27, 2026

    doc: add documentation for http.IncomingMessage$complete

    bd8c107

Issue actions