Skip to content

socket[Symbol.asyncIterator]() does not complete in v10.14.2+ #28852

Closed

Description

@braddunbar
  • Version: v10.14.2
  • Platform: Darwin anahola 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64
  • Subsystem: stream

In node v10.14.1, an asyncIterator over an upgraded HTTP socket completes after being closed from the client. In node v10.14.2, it does not.

The test case:

const http = require('http')

const server = http.createServer(() => {})

server.on('upgrade', async (request, socket) => {
  socket.write([
    'HTTP/1.1 101 Switching Protocols',
    'upgrade: websocket',
    'connection: upgrade',
    '\r\n'
  ].join('\r\n'))

  for await (const message of socket[Symbol.asyncIterator]()) {
    console.log(message.toString())
  }

  console.log('done')

  socket.destroy()
  server.close()
})

server.listen(() => {
  const { port, family } = server.address()

  const request = http.request({
    port,
    family: family === 'IPv6' ? 6 : 4,
    headers: {
      connection: 'upgrade',
      upgrade: 'websocket'
    }
  })

  request.on('upgrade', (response, socket) => { socket.end('sent') })
  request.end('')
})

In v10.14.1, this is the output followed by the process exiting.

$ nvm exec 10.14.1 node case.js
Running node v10.14.1 (npm v6.4.1)
(node:89863) ExperimentalWarning: Readable[Symbol.asyncIterator] is an experimental feature. This feature could change at any time
sent
done
$ _

In v10.14.2, only the first line is printed and the process never exits.

$ nvm exec 10.14.2 node case.js
Running node v10.14.2 (npm v6.4.1)
(node:90228) ExperimentalWarning: Readable[Symbol.asyncIterator] is an experimental feature. This feature could change at any time
sent

Activity

  1. added
    httpIssues and PRs related to the http subsystem.
    on Jul 25, 2019
  2. braddunbar commented on Jul 25, 2019

    @braddunbar
    Author

    As it turns out, this is fixed in 12.7.0! 🎉

    I'm pretty sure it was #28566 that fixed it. Thanks @davidmarkclements!

  3. braddunbar commented on Jul 25, 2019

    @braddunbar
    Author

    Yep, verified by reverting 0793398 which makes the test case above fail. 🙌

  4. davidmarkclements commented on Jul 25, 2019

    @davidmarkclements
    Member

    Oh wow cool - I should have tried latest - apologies
    lol sorry I got confused with issues - cool, glad it solved it!

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