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

feat(ext/http): Show that is also listening on localhost #28171

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

WasixXD
Copy link

@WasixXD WasixXD commented Feb 18, 2025

Closes #27722

This will show Listening on http://0.0.0.0:8000 (accessible via http://localhost:8000)

@CLAassistant
Copy link

CLAassistant commented Feb 18, 2025

CLA assistant check
All committers have signed the CLA.

@@ -740,7 +740,7 @@ function serve(arg1, arg2) {
}

const listenOpts = {
hostname: options.hostname ?? "0.0.0.0",
hostname: options.hostname ?? "localhost",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fail a bunch of tests. It might be good to make this a display only thing.

@devsnek
Copy link
Member

devsnek commented Feb 19, 2025

This isn't equivalent at all. 0.0.0.0 is all interfaces, while localhost (127.0.0.1) is only one ip on one loopback interface. I guess, based on the linked issue, you want to make it more user friendly to people who don't understand this routing behavior. Maybe you can test for the socket address being 0.0.0.0 or :: and output an additional bit of text (while still being clear that its listening on 0.0.0.0)

@WasixXD
Copy link
Author

WasixXD commented Feb 19, 2025

Thanks for the insights, I'll redo it

@WasixXD
Copy link
Author

WasixXD commented Feb 20, 2025

This isn't equivalent at all. 0.0.0.0 is all interfaces, while localhost (127.0.0.1) is only one ip on one loopback interface. I guess, based on the linked issue, you want to make it more user friendly to people who don't understand this routing behavior. Maybe you can test for the socket address being 0.0.0.0 or :: and output an additional bit of text (while still being clear that its listening on 0.0.0.0)

With additional bit of text you mean like:

Listening on http://0.0.0.0:8000 (accessible via http://localhost:8000)

?

Or just showing Listening on http://localhost:8000 should be enough and I just correct the tests cases?

@devsnek
Copy link
Member

devsnek commented Feb 20, 2025

Listening on http://0.0.0.0:8000/ (accessible via http://localhost:8000/)

This seems like the ideal behavior to me, if you're willing to implement it.

@WasixXD WasixXD changed the title feat(ext/http): Show localhost instead of 0.0.0.0 feat(ext/http): Show that is also listening on localhost Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deno --init serve: http://0.0.0.0:8000 does not work with Safari
4 participants