nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.6k stars 29.59k forks source link

Inconsistent http stream behavior #33672

Open tadjik1 opened 4 years ago

tadjik1 commented 4 years ago

Hey there,

What steps will reproduce the bug?

We have web-server and we want to limit the amount of data that can be sent by the client. In the code below, I've tried to implement such a server (it counts the number of chunks) and client that sends a stream of infinity size to the server. I assume that after the 10th chunk server should send the response to the client which will lead to 2 things on the client-side: error event on request object (because of premature stream close) and response callback been called (because the server sends a response to the client).

However, it turns out that this code prints EPIPE error in 100% of cases which is correct, but it prints response only in ~50% of cases.

If you change res.end() to res.end("something") (so that response will contain a body) then it will always print error and response, so the client will always be able to get a response from the server.

const http = require('http');
const { Readable } = require('stream');

const server = new http.Server();

server.on('request', (req, res) => {
  let i = 0;
  req.on('data', chunk => {
    i++;
    if (i === 10) {
      res.statusCode = 413;
      res.end(); // try to change it to res.end("too big")
    }
  });
});

server.listen(3000, makeRequest);

function makeRequest() {
  const request = http.request(
    'http://localhost:3000',
    {method: 'POST'},
    response => {
      console.log("response finished", response.statusCode);
      server.close();
    }
  );

  request.on('error', err => {
    console.log("request error", err);
  });

  Readable.from(generate()).pipe(request);
}

function* generate() {
  while(true) { yield "value"; }
}

What is the expected behavior?

I expect to see consistent behavior, so the client should be able to always get a response.

lundibundi commented 4 years ago

Cannot reproduce on Linux (ArchLinux), there is always at least response finished 413 console message however many times I run this.

lundibundi commented 4 years ago

ping @nodejs/http

rickyes commented 4 years ago

I'm working on this.

tadjik1 commented 4 years ago

@rickyes what is the problem here? Maybe I can help you with it if you share some insights about the root cause and potential way to fix it that you have in mind.

rickyes commented 4 years ago

It seems that HTTPParser did not trigger the kOnHeadersComplete method, and even if include a body to end the response res.end('something') will have the same problem, but with a small chance of occurring.

tadjik1 commented 4 years ago

@rickyes I thought it might be that 'error' on writing is coming first and reading part is simply ignore by net/http/stream (not sure which) implementation. Do you know the way how this can be checked or tested?

rickyes commented 4 years ago

I can't find out why this error happened, but maybe we can get more information from @ronag @mcollina @nodejs/http, PTAL.

mcollina commented 4 years ago

The reason why we are seeing this non-deterministic behavior is because the socket can be destroyed before or after the headers are completed while the request. The socket is shared between the request and response object in the client and it will be destroyed if it receives a response before it has finished sending all the request.

tadjik1 commented 4 years ago

@mcollina thanks a lot for joining this issue and shed light on this problem. If I understand correctly the socket receives the response, in this case, so it should be provided to the user (in "response" event handler).

mcollina commented 4 years ago

'response' is emitted when the headers have been parsed. https://nodejs.org/api/http.html#http_event_socket is emitted when you get a socket.

The problem is in your example. The flow of succesfull http request/response is the following:

  1. the client sends the headers and start sending the body
  2. the server parses the headers, it might start responding.
  3. the client finished to transmit the body
  4. the server closes its response.

Anything else than this can cause problems.