Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http destroys socket too soon #21026

Closed
rogierschouten opened this issue May 30, 2018 · 3 comments
Closed

http destroys socket too soon #21026

rogierschouten opened this issue May 30, 2018 · 3 comments
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. http Issues or PRs related to the http subsystem.

Comments

@rogierschouten
Copy link

  • Version: 10.2.1
  • Platform: Windows 10 64-bit
  • Subsystem: http

Problem description:
When an HTTP server decides to send a response while the request body is still being sent, there are cases (race condition) where the client receives an ECONNRESET error without receiving the response.

Example:

const http = require("http");
const net = require("net");
const stream = require("stream");

const PORT = 1235;

let serverReceivedRequestHeaders = false;

// create HTTP server
const server = http.createServer((request, response) => {
	serverReceivedRequestHeaders = true;
	response.writeHead(401, "test error");
	response.end();
});
server.listen(PORT, (error) => {
	if (error) {
		console.log(error);
		return;
	}

	// send request
	const request = http.request({
		hostname: 'localhost',
		method: 'POST',
		path: '/',
		port: PORT
	});
	request.once("response", (response) => {
		console.log("response", response.statusCode);
	});
	request.once("error", (error) => {
		console.log("request error", error);
	});

	// keep sending more request body until the server receives the request headers
	const requestBody = new stream.PassThrough();
	const writeMore = (cb) => {
		requestBody.write("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", (error) => {
			if (error) {
				cb(error);
			} else if (serverReceivedRequestHeaders) {
				cb();
			} else {
				writeMore(cb);
			}
		});
	}
	requestBody.pipe(request);
	writeMore(() => undefined);
});

Expected output:

response 401

Actual output:

request error { Error: read ECONNRESET
    at TCP.onread (net.js:657:25) errno: 'ECONNRESET', code: 'ECONNRESET', syscall: 'read' }

Notes:

  • It's clearly a race condition because e.g. removing the pass-through stream and directly writing to the request object does not reproduce the problem.
  • Node 9.4.0 behaves slightly differently; it has the same problem but there it does NOT get resolved by removing the pass-through stream
@bnoordhuis
Copy link
Member

Is this a duplicate of #12339?

@bnoordhuis bnoordhuis added the http Issues or PRs related to the http subsystem. label May 30, 2018
@rogierschouten
Copy link
Author

@bnoordhuis that looks similar indeed. And in my case it's quite serious since I have to talk to a server that has this behavior and it's not under my control. And I do need that response.

@bnoordhuis
Copy link
Member

Okay, I'll close this out as a duplicate. You should chime in on the other issue but as you can probably tell from the discussion, it's not clear what the solution should be.

@bnoordhuis bnoordhuis added the duplicate Issues and PRs that are duplicates of other issues or PRs. label May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants