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

'error' event no longer emitted for net connections #43724

Closed
jonathansamines opened this issue Jul 7, 2022 · 3 comments
Closed

'error' event no longer emitted for net connections #43724

jonathansamines opened this issue Jul 7, 2022 · 3 comments
Labels
net Issues and PRs related to the net subsystem.

Comments

@jonathansamines
Copy link
Contributor

Version

16.15.1

Platform

Darwin LM-GUA-42000176 20.6.0 Darwin Kernel Version 20.6.0: Tue Apr 19 21:04:45 PDT 2022; root:xnu-7195.141.29~1/RELEASE_X86_64 x86_64

Subsystem

net

What steps will reproduce the bug?

'use strict';

const net = require('net');
const port = 4500;

const server = net.createServer((socket) => {
    socket.on('end', writeMessage);
    socket.end();
    server.close();
});

server.listen(port);

const socket = net.createConnection(port);

function writeMessage() {
    function logWriteError(err) {
        console.log('write cb error: ', err);
    }

    function logEventError(err) {
        console.log('event error: ', err);
    }

    socket.on('error', logEventError);
    socket.write('some message', 'utf8', logWriteError);
}

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

Always

What is the expected behavior?

The 'error' event is emitted when .write() is called after a connection has been terminated by the server. This is how it used to work in Node 14:

event error:  Error: This socket has been ended by the other party
    at Socket.writeAfterFIN [as write] (net.js:468:14)
    at Socket.writeMessage (/Users/jsamines/dev/personal/node-lazy-socket/test/integration/test-net-connect-close-errors.js:26:12)
    at Socket.emit (events.js:412:35)
    at endReadableNT (internal/streams/readable.js:1333:12)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  code: 'EPIPE'
}
write cb error:  Error: This socket has been ended by the other party
    at Socket.writeAfterFIN [as write] (net.js:468:14)
    at Socket.writeMessage (/Users/jsamines/dev/personal/node-lazy-socket/test/integration/test-net-connect-close-errors.js:26:12)
    at Socket.emit (events.js:412:35)
    at endReadableNT (internal/streams/readable.js:1333:12)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  code: 'EPIPE'
}

What do you see instead?

The 'error' event is no longer emitted when .write() is called after a connection has been terminated by the server on Node 16:

write cb error:  Error: This socket has been ended by the other party
    at Socket.writeAfterFIN [as write] (node:net:487:14)
    at Socket.writeMessage (/Users/jsamines/dev/personal/node-lazy-socket/test/integration/test-net-connect-close-errors.js:26:12)
    at Socket.emit (node:events:539:35)
    at endReadableNT (node:internal/streams/readable:1345:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  code: 'EPIPE'
}

Additional information

It might be an intentional breaking change introduced by #31806, though it is hard to tell if this specific change is intentional. The provided reproduction is as small as possible, but this change in behavior broke lazy-socket (See felixge/node-lazy-socket#3), which in turn broke the graphite library (See felixge/node-graphite#16).

@daeyeon daeyeon added the net Issues and PRs related to the net subsystem. label Jul 7, 2022
@benjamingr
Copy link
Member

@nodejs/streams @nodejs/net

@mcollina
Copy link
Member

mcollina commented Jul 8, 2022

This change was deliberate. Unfortunately, v14 is in maintenance and v16 is in LTS, so I think this behavior is now stable and even if we wanted to, we won't be able to change it back before v19.

I propose we close this issue as won't fix: those libraries would need to be updated as they are essentially unmaintained.

@jonathansamines
Copy link
Contributor Author

Thanks for confirming @mcollina. Just wanted to confirm the change was intentional before proposing changes to those libraries.

@jonathansamines jonathansamines closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants