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

net socket does not behave according to stream documentation after close #53512

Open
joepie91 opened this issue Jun 19, 2024 · 3 comments
Open
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@joepie91
Copy link
Contributor

Version

v18.20.2

Platform

Linux desktop-home 6.6.33 #1-NixOS SMP PREEMPT_DYNAMIC Wed Jun 12 09:13:03 UTC 2024 x86_64 GNU/Linux

Subsystem

net

What steps will reproduce the bug?

"use strict";

const net = require("net");

let server = net.createServer((socket) => {
	socket.on("close", () => {
		console.log("socket fully closed");
	});

	let success = socket.write("test");
	console.log({ success }); // { success: true }

	setTimeout(() => {
		// This *should* throw an error, according to streams documentation
		let success = socket.write("test");
		console.log({ success }); // instead, it logs { success: false }
	}, 2000);
});

server.listen(4891);

let client = net.connect(4891, () => {
	client.end();
});

client.on("data", (data) => {
	console.log("recv:", data);
});

/* Full output:
{ success: true }
recv: <Buffer 74 65 73 74>
socket fully closed
{ success: false }
*/

How often does it reproduce? Is there a required condition?

Consistently happens.

What is the expected behavior? Why is that the expected behavior?

After a socket has been closed by the other side, attempting to write to it should result in a write error; a net.Socket is a stream.Writable, which says the following in the documentation:

Calling the stream.write() method after calling stream.end() will raise an error.

Although in the most literal interpretation this only specifies that an error would occur if end was called on the same side that you are attempting to write from, it is surprising behaviour that this is different when the socket is closed from the other side, because the writable stream has still ended all the same (and the above reduced testcase demonstrates this by logging the 'socket fully closed' event).

What do you see instead?

Instead, the write-while-closed merely returns false as if the internal buffer has temporarily filled up, but then never actually emits a drain event (presumably because there is no socket to drain to anymore).

Additional information

I worked this out after several hours of debugging, and I was unable to find any reference to this behaviour in the documentation; in my case, it created a difficult-to-debug silent failure, as my code was waiting for the drain signal to continue writing data, but never received it.

@lpinca
Copy link
Member

lpinca commented Jun 19, 2024

This is by design. No 'error' event is emitted after the 'close' event.

const { Writable } = require('stream');

const writable = new Writable({
  write(chunk, encoding, callback) {
    callback();
  }
});

writable.on('close', function () {
  const ret = writable.write('test');
  console.log(ret);
});

writable.end();

However, the writable.write() callback is still called with an error.

@joepie91
Copy link
Contributor Author

That is very confusing behaviour, especially as the writable.end documentation makes no mention of the "throws an error when attempting to write" only occurring during a very limited window (between the end call and the close event, as it seems).

And the result is that a write to a 'dead' stream (that will never drain) is indistinguishable from a write to a backlogged stream (that should drain eventually), when implementing backpressure according to the documentation - the documentation of writable.write makes no mention of this dual meaning either.

What's the rationale for not emitting any error events after close?

@lpinca
Copy link
Member

lpinca commented Jun 19, 2024

No event is emitted after 'close', it is the last event. When 'close' is emitted the stream is destroyed. An 'error' event may have already been emitted before 'close'. Emitting it again would result in multiple 'error' events.

@lpinca lpinca added the stream Issues and PRs related to the stream subsystem. label Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants