http.IncomingMessage omits events when aborted on client side

[skenqbx]

• Version: 6.3.0
• Platform: Linux offline 3.19.0-64-generic Is V8 the best JS engine today? #72-Ubuntu SMP Fri Jun 24 15:12:52 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: http

When a HTTP client sends a partial body and is then aborted, the corresponding server request object does not emit end, close or aborted events. Only a clientError is emitted on the server.

I looked at the existing tests concerning request abortion and it seems this is the only case that is untested.

The following code reproduces the problem:

'use strict';
const http = require('http');


const server = http.createServer((request, response) => {
  console.log('server request');

  request.on('aborted', () => console.log('server request aborted'));
  request.on('error', (err) => console.log('server request error', err));

  request.on('readable', () => console.log('server request readable', request.read()));
  request.on('end', () => console.log('server request end'));

  request.on('close', () => {
    console.log('server request close');
    server.close();
  });
});

server.on('clientError', (err) => {
  console.log('server clientError', err);
});

server.listen(0, () => {
  const port = server.address().port;

  const request = http.request({
    port: port,
    path: '/',
    method: 'PUT'
  }, (response) => console.log('client response'));

  request.on('abort', () => console.log('client abort'));
  request.on('error', (err) => console.log('client error', err));

  request.write('Test');

  setTimeout(() => {
    request.abort();
  }, 100);
});

The output shows that the readable stream (request) is left waiting for the end event

server request
server request readable <Buffer 54 65 73 74>
client abort
server clientError { Error: Parse Error
    at Error (native)
    at Socket.socketOnEnd (_http_server.js:422:22)
    at emitNone (events.js:91:20)
    at Socket.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:973:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9) bytesParsed: 0, code: 'HPE_INVALID_EOF_STATE' }
client error { Error: socket hang up
    at createHangUpError (_http_client.js:252:15)
    at Socket.socketCloseListener (_http_client.js:284:23)
    at emitOne (events.js:101:20)
    at Socket.emit (events.js:188:7)
    at TCP._handle.close [as _onclose] (net.js:492:12) code: 'ECONNRESET' }

[cjihrig]

Are those events expected to be emitted when a clientError occurs? Based on the documentation for clientError and the existing tests, I would think not.

[skenqbx]

In this specific case, where the server's requestListener has already been invoked, I would expect the request object to emit at least the close event, so I can cleanup after the request.

[cjihrig]

The clientError event handler also passes in the socket as a second argument. By listening for clientError events, you are opting in to closing/destroying the socket (per the documentation). If you change your clientError handler to:

server.on('clientError', (err, socket) => {
  console.log('server clientError', err);
  socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
});

This will cause the request object's abort and close methods to be emitted, which will let you clean up.

I have to say, the docs could probably be improved here for clarity. Also, based on your example, this statement in the docs:

When the 'clientError' event occurs, there is no request or response object...

Appears to be at least partially wrong.

[cjihrig]

Also ping @nodejs/http if you think I'm missing something here.

[skenqbx]

I did overlook that, thank you for the help!
Shall I send a PR for the docs?

[cjihrig]

If you're feeling up to it, sure.

[indutny]

I can't seem to be able to reproduce it:

server request
server request readable <Buffer 54 65 73 74>
server request aborted
server request close
client error { [Error: socket hang up] code: 'ECONNRESET' }
~/Code/indutny/node $ ./node 1
server request
server request readable <Buffer 54 65 73 74>
client abort
Trace
    at Socket.destroy (net.js:519:13)
    at Socket.socketOnEnd (_http_server.js:425:14)
    at emitNone (events.js:72:20)
    at Socket.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:921:12)
    at nextTickCallbackWith2Args (node.js:442:9)
    at process._tickCallback (node.js:356:17)
Trace
    at Socket.destroy (net.js:519:13)
    at Server.<anonymous> (_http_server.js:244:10)
    at emitTwo (events.js:92:20)
    at Server.emit (events.js:172:7)
    at Socket.socketOnError (_http_server.js:359:10)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at emitErrorNT (net.js:1278:8)
    at nextTickCallbackWith2Args (node.js:442:9)
    at process._tickCallback (node.js:356:17)
server clientError { [Error: Parse Error] bytesParsed: 0, code: 'HPE_INVALID_EOF_STATE' }
server request aborted
server request close
client error { [Error: socket hang up] code: 'ECONNRESET' }
~/Code/indutny/node $

[cjihrig]

FWIW, I'm able to reproduce the behavior as reported with master on OS X.

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.