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

fix: Honor the host param when creating a websocket server #5617

Merged
merged 1 commit into from Mar 3, 2022

Conversation

casret
Copy link
Contributor

@casret casret commented Nov 10, 2021

Description

This is similar to the 3c53f82 that fixed #2032 but for the websocket server.


What is the purpose of this pull request?

  • [ x ] Bug fix

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@casret casret changed the title Honor the host param when creating a websocket server fix: Honor the host param when creating a websocket server Nov 10, 2021
@patak-dev
Copy link
Member

LGTM. I don't know if a test case could be added, but would it be possible to get a reproduction failing for this case?

@casret
Copy link
Contributor Author

casret commented Nov 10, 2021

While my actual situation involves node binding to the wrong ipv4/ipv6 stack, I would expect this commit casret@8edbb1e to raise an error since we can't bind to '8.8.8.8', but it runs fine.

On my branch it will raise:

Error: listen EADDRNOTAVAIL: address not available 8.8.8.8:24678
    at Server.setupListenHandle [as _listen2] (net.js:1301:21)
    at listenInCluster (net.js:1366:12)
    at doListen (net.js:1503:7)
    at processTicksAndRejections (internal/process/task_queues.js:81:21)

@casret
Copy link
Contributor Author

casret commented Dec 2, 2021

@patak-js Anything I can do to get this merged?

@casret casret mentioned this pull request Dec 2, 2021
7 tasks
@casret
Copy link
Contributor Author

casret commented Dec 30, 2021

@patak-dev ping?

@patak-dev
Copy link
Member

Sorry for the delay, lets get this one in Vite 2.8

@casret
Copy link
Contributor Author

casret commented Feb 9, 2022

@patak-dev Noticed that it didn't make it in to 2.8? Is there something else I need to patch up?

@Niputi Niputi added this to the 2.9 milestone Feb 9, 2022
@patak-dev
Copy link
Member

Sorry @casret, we should have added it to the 2.8 milestone. Thanks @Niputi for adding it to the 2.9 milestone so we don't miss it again. There are already several PRs queued and we just released 2.8 today so I think we may start the next beta quite soon.

@patak-dev patak-dev merged commit 882c8a8 into vitejs:main Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

vite server listen on IPv6 only
4 participants