-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
Description
- Version: v12.16.3
- Platform: Linux 73659abd8e3f 4.19.76-linuxkit deps: update openssl to 1.0.1j #1 SMP Thu Oct 17 19:31:58 UTC 2019 x86_64 GNU/Linux
- Subsystem: _http_agent, _http_client
What steps will reproduce the bug?
const http = require('http')
const { host = '127.0.0.1', port = '8080' } = process.env
http
.createServer((_, res) => {
res.end('data')
})
.listen(port, () => {
const request = http
.request({
host,
port,
headers: { 'Connection': 'keep-alive' }
}, response => {
response
.on('data', () => {})
.on('end', () => {})
})
.end()
setTimeout(() => {
request.socket.destroy('Some error') // this line causes the error
}, 500)
})How often does it reproduce? Is there a required condition?
It is reproduced 100% when running the code snippet above with node v12.16.3.
What is the expected behavior?
The behavior is inconsistent between node v12.16.2 and v12.16.3. I suppose, it would be ok in a next major-version. But, as it is a patch-version release, the error should not be thrown.
What do you see instead?
events.js:301
throw err; // Unhandled 'error' event
^
Error [ERR_UNHANDLED_ERROR]: Unhandled error. ('Some error')
at Socket.emit (events.js:299:17)
at emitErrorNT (internal/streams/destroy.js:92:8)
at processTicksAndRejections (internal/process/task_queues.js:84:21) {
code: 'ERR_UNHANDLED_ERROR',
context: 'Some error'
}
Additional information
Looks like the problem is caused by this commit 225ddd5, it's exactly what @ronag described here. The socket has no 'error' event handler, so unhandled error is thrown, which can only be caught with process.on. Otherwise the process crashes.
I'm not sure if it's a heavy breaking change or the commit I mentioned should be reverted, as it can easily be fixed by adding an 'error' event handler to a socket. But it definitely caused me an hour or two of happy debug.
Please let me know if I can help with those todos in _http_client.js.