-
-
Notifications
You must be signed in to change notification settings - Fork 7
Update port listening from '0.0.0.0' to '::' #470
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| const start = async (): Promise<void> => { | ||
| try { | ||
| await app.listen({host: '0.0.0.0', port}); | ||
| await app.listen({host: '::', port}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: IPv6-Only Bind Breaks IPv4-Only Environments
Changing the listen host from '0.0.0.0' to '::' breaks compatibility with IPv4-only systems and environments without IPv6 support. While '::' works on IPv6-enabled systems (and often accepts IPv4 connections via IPv4-mapped IPv6 addresses on dual-stack systems), it will fail on systems where IPv6 is disabled or not supported, making the application unable to start in those environments. This is a breaking change that trades fixing IPv6-only platforms (like Fly) at the cost of breaking IPv4-only deployments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not realise it would break on non-IPv6 environments, my apologies, I guess there could be an env var set to enable IPv6 support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @icepcp — I'd welcome a PR that uses an environment variable for the host 👍
cmraible
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
To avoid any potential breaking changes, we should make this configurable via an environment variable, and default to the current '0.0.0.0'. Happy to take another look if you want to update this PR to use the environment variable.
Docker does not necessarily use IPv6 for the internal network by default, which allows for listening on IPv4 addresses. Hosting on external platforms, like Fly, where its internal network is strictly IPv6, leads to errors when trying to use TrafficAnalytics. Updating the port to be listening on :: would solve this issue
Note
Change server listen host to IPv6
::instead of IPv40.0.0.0.Written by Cursor Bugbot for commit 682fd21. This will update automatically on new commits. Configure here.