Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

http.ClientResponse should have an error event #3375

Closed
markdaws opened this issue Jun 5, 2012 · 7 comments
Closed

http.ClientResponse should have an error event #3375

markdaws opened this issue Jun 5, 2012 · 7 comments

Comments

@markdaws
Copy link

markdaws commented Jun 5, 2012

Currently if there is an error parsing the response from a server e.g. the server sends back an incorrect value in the content length, this will result in an unhandledException unless you add the following code:

req.on('socket', function(socket) {
    socket.on('error', function(error) {
        //do something
    });
});

There should just be an 'error' event on the http.ClientResponse object that you can easily listen to in order to catch the error instead of having to go down to the socket level and hook up there.

@isaacs
Copy link

isaacs commented Jun 5, 2012

ClientResponse does emit an error event in this case, doesn't it?

var req = http.request(options, function (res) {
  res.on('error', handler)
})

@markdaws
Copy link
Author

markdaws commented Jun 5, 2012

The documentation doesn't show an error event in the response object: http://clipboard.com/clip/LQhHMG1qw4PaDdSPeTPKaYCzHQMELRCijRLe I added one just incase but it didn't get raised. For example on this head request:

~$ curl -I "http://pressebo.ivwbox.de/cgi-bin/ivw/CP/MainIndex;?r=&d=11727.789742872119"
HTTP/1.1 302 FOUND
Server: s2.31.1
Date: Tue, 05 Jun 2012 19:28:04 GMT
Last-Modified: Tue, 22 Aug 2000 15:05:01 GMT
Pragma: no-cache
Cache-Control: no-cache, must-revalidate
Expires: Tue, 22 Aug 2000 15:05:01 GMT
P3P: policyref="http://www.ivwbox.de/p3p.xml", CP="NOI DSP PSAo OUR NOR UNI"
Set-Cookie: srp=016e4fce5dc4d1de0007; path=/
Location: /blank.gif
Content-Length: 1
Content-Type: text/plain

The content length is wrong, resulting in a Parse Error which only seems to be able to be caught if you hook up to the socket error.

@markdaws
Copy link
Author

markdaws commented Jun 5, 2012

Ah - I see ClientResponse implements ReadableStream that does have an error event. However it looks like if you set the options.method to HEAD then the error is not raised on the ClientResponse object. Here is a repro:

var http = require('http');
var options = require('url').parse('http://pressebo.ivwbox.de/cgi-bin/ivw/CP/MainIndex;?r=&d=11727.789742872119');

console.dir(options);

// crashes on a HEAD request                                                                                                                                                                                      
options.method = 'HEAD';

var req = http.request(options, function (res) {
        console.log('response:' + res.statusCode);

        res.on('error', function(error) {
                console.log('Error:' + error);
        });
});
req.end();

OUTPUT:
{ protocol: 'http:',
slashes: true,
host: 'pressebo.ivwbox.de',
hostname: 'pressebo.ivwbox.de',
href: 'http://pressebo.ivwbox.de/cgi-bin/ivw/CP/MainIndex;?r=&d=11727.789742872119',
search: '?r=&d=11727.789742872119',
query: 'r=&d=11727.789742872119',
pathname: '/cgi-bin/ivw/CP/MainIndex;',
path: '/cgi-bin/ivw/CP/MainIndex;?r=&d=11727.789742872119' }
response:302

events.js:48
throw arguments[1]; // Unhandled 'error' event
^
Error: Parse Error
at Socket.socketOnData (http.js:1288:20)
at TCP.onread (net.js:374:27)

@isaacs
Copy link

isaacs commented Jun 5, 2012

Ah. Yes, that's a bug. Confirmed.

@isaacs
Copy link

isaacs commented Jun 6, 2012

Confirmation test:

var http = require('http');
var net = require('net');

var server = net.createServer(function(sock) {
  sock.end(
    'HTTP/1.1 200 OK\r\n' +
    'Date: ' + new Date().toUTCString() + '\r\n' +
    'Content-Length: 1\r\n' +  // wrong length
    'Content-Type: text/plain\r\n' +
    '\r\n' +
    '\r\n' +
    'This is too long.\r\n' +
    '\r\n' +
    '\r\n');
  sock.destroySoon();
  server.close();
});

server.listen(1337);

var options = { host: 'localhost',
                port: 1337,
                path: '/',
                method: 'HEAD' };

var req = http.request(options, function (res) {
  req.hadResponse = true;
  res.hadResponse = true;
  console.error('response:', res.statusCode, res.headers);

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

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

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

req.end();

Expect: response error.
Actual: socket error.

@koichik
Copy link

koichik commented Jun 6, 2012

Refs #3354.

the server sends back an incorrect value in the content length

Node (http-parser) cannot detect it. See section "4.4 Message Length" of RFC2616. Then, Node detects invalid data after the response body. (i.e. The response is completed definitely.)
So, I do not think that 'error' event should be emitted on ClientResponse. However, it should be catchable. (on Agent?)

@koichik
Copy link

koichik commented Aug 28, 2012

Closed by #3777

@koichik koichik closed this as completed Aug 28, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants