Skip to content

Calling socket.destroy() causes unhandled error in v12.16.3 #33434

Description

@pkozemirov
  • 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.

Activity

  1. ronag commented on May 18, 2020

    @ronag
    Member

    I will try to help out here later this week. Feel free to ping me if I forget.

  2. vostrik commented on May 27, 2020

    @vostrik
    Member

    @ronag should my colleague @pkozemirov and I do PR to fix this behaviour?

  3. ronag commented on May 27, 2020

    @ronag
    Member

    @vostrik go for it! I'll be happy to review

  4. Baxtishodd commented on May 27, 2020

    @Baxtishodd
  5. ronag commented on May 27, 2020

    @ronag
  6. added
    httpIssues and PRs related to the http subsystem.
    on Dec 27, 2020
  7. targos commented on Dec 27, 2020

    @targos
    Member

    Was this fixed?

  8. brucewar commented on Apr 6, 2022

    @brucewar

    +1

  9. targos commented on Apr 8, 2022

    @targos
    Member

    Closing this issue because v12.x isn't going to get new releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    httpIssues and PRs related to the http subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions