node 16.x does not raise error on write when server closes connection

[tlbdk]

Node version and platform

$ node -v
v16.0.0
$ uname -a
Darwin Troelss-Mac-mini.local 20.4.0 Darwin Kernel Version 20.4.0: Thu Apr 22 21:46:41 PDT 2021; root:xnu-7195.101.2~1/RELEASE_ARM64_T8101 arm64

What steps will reproduce the bug?

let os = require('os')
let net = require('net')
let crypto = require('crypto')

let socketPath = `${os.tmpdir()}/${crypto.randomBytes(12).toString('hex')}.sock`

let server = net.createServer(socket => {
    let buffer = Buffer.alloc(0)
    socket.on('data', chunk => {
        if(chunk.toString('utf8') === 'destroy') {
            console.log('server destroy')
            socket.destroy()
        }       
    })
})
server.on('listening', () => {
    // client closes
    const socket1 = net.connect({ path: socketPath })
    socket1.on('error', (e) => {
        console.log('socket1:' + e.toString())
    })
    socket1.end()
    socket1.write('hello')

    // server closes
    const socket2 = net.connect({ path: socketPath })
    socket2.on('error', (e) => {
        console.log('socket2:' + e.toString())
    })
    socket2.write('destroy')
    setTimeout(() => {
        socket2.write()
    }, 1000)
})
server.listen(socketPath)

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

Every time.

What is the expected behavior?

Node 12.x:

$ /opt/homebrew/Cellar/node@12/12.22.1_1/bin/node socket-errors.js
socket1:Error [ERR_STREAM_WRITE_AFTER_END]: write after end
server destroy
socket2:Error: This socket has been ended by the other party

Node 14.x:

$ /opt/homebrew/Cellar/node@14/14.17.0/bin/node socket-errors.js 
socket1:Error [ERR_STREAM_WRITE_AFTER_END]: write after end
server destroy
socket2:Error: This socket has been ended by the other party

What do you see instead?

Node 16.x:

$ /opt/homebrew/Cellar/node/16.0.0_1/bin/node socket-errors.js 
socket1:Error [ERR_STREAM_WRITE_AFTER_END]: write after end
server destroy

Additional information

[targos]

@nodejs/streams ?

[ronag]

@nodejs/net

[mcollina]

This behavior is correct and wanted, and it is the combination of a few PRs that landed in v15.

What is happening is:

1. the server destroy() the connection.
2. the client emit 'close', no more errors should be emitted.
3. after 1 second, the client try sending some data. No error is emitted, but it is passed to the .write() callback.

In your client example, you are not waiting 1 second before sending the .write() message and the socket is not closed yet.

Here is a complete example:

let os = require('os')
let net = require('net')
let crypto = require('crypto')

let socketPath = `${os.tmpdir()}/${crypto.randomBytes(12).toString('hex')}.sock`

let server = net.createServer(socket => {
    let buffer = Buffer.alloc(0)
    socket.on('data', chunk => {
        console.log('chunk', chunk.toString())
        if(chunk.toString('utf8') === 'destroy') {
            console.log('server destroy')
            socket.destroy()
        }
    })
})
server.on('listening', () => {
    // client closes
    const socket1 = net.connect({ path: socketPath })
    socket1.on('error', (e) => {
        console.log('socket1:' + e.toString())
    })
    socket1.end()
    setTimeout(function () {
      socket1.write('hello', function (err) {
        console.log('hello written', err)
      })
    }, 1000)

    // server closes
    const socket2 = net.connect({ path: socketPath })
    socket2.on('error', (e) => {
        console.log('socket2:' + e.toString())
    })
    socket2.write('destroy')
    setTimeout(() => {
        socket2.write('something', function (err) {
          console.log('something written', err)
        })
    }, 1000)
    socket2.on('end', () => {
      console.log('socket2 ended')
    })
    socket2.on('close', () => {
      console.log('socket2 closed')
    })
})
server.listen(socketPath)

[tlbdk]

@mcollina I understand the reasoning and it is more consistent, the documentation could be a bit more clear that the callback for socket.write callback will returns an error as this quite important to avoid silently loosing the write.

[mcollina]

@mcollina I understand the reasoning and it is more consistent, the documentation could be a bit more clear that the callback for socket.write callback will returns an error as this quite important to avoid silently loosing the write.

Agreed.

https://nodejs.org/api/stream.html#stream_writable_write_chunk_encoding_callback looks quite confusing in this regard.

[mcollina]

Would you like to send a pull request?

[inidaname]

Hello @mcollina does this still need a PR, I would love to take a shot at this

[mcollina]

Yes it does, it's a doc update.

[inidaname]

could you point me to the doc file. thank you

[mcollina]

could you point me to the doc file. thank you

https://github.com/nodejs/node/blob/master/doc/api/stream.md