Websocket for LiveReload using wrong port if Hugo binds to port 80 #2205

Closed
chuyskywalker opened this Issue Jun 13, 2016 · 10 comments

Projects

None yet

3 participants

@chuyskywalker
chuyskywalker commented Jun 13, 2016 edited

Let's say I clone this repo:

git clone https://github.com/robertbasic/robertbasic.com-hugo.git

Then start up a docker container on Hugo 0.16:

docker run -ti --rm --name "my-hugo" \
  -p 80:80 -v $(pwd)/robertbasic.com-hugo:/src jojomi/hugo \
  hugo server --watch=true --source="/src" --bind=0.0.0.0 --port=80 --baseURL="/"

Where I have watch on and I'm listening on port 80. When you load the site, you get:

WebSocket connection to 'ws://192.168.51.1:35729/livereload' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED

Which is clearly trying to connect to the hugo server on the wrong port. If you pop the port number up one value:

docker run -ti --rm --name "my-hugo" \
  -p 81:81 -v $(pwd)/robertbasic.com-hugo:/src jojomi/hugo \
  hugo server --watch=true --source="/src" --bind=0.0.0.0 --port=81 --baseURL="/"

It works as expected (from chrome console):

Request URL:ws://192.168.51.1:81/livereload
Request Method:GET
Status Code:101 Switching Protocols

Originally discussed and discovered here: https://discuss.gohugo.io/t/live-reload-unconnectable-in-docker-container/3457/4

@bep
Collaborator
bep commented Jun 13, 2016

hugo server is a development server. Why do you need it to bind to port 80?

@chuyskywalker

hugo server is a development server. Why do you need it to bind to port 80?

I like typing less stuff in my url bar? "Because."? Does it matter? Why shouldn't I use port 80? There's a config option for it and in certain situations it causes other functionality to fail.

The port I choose for hugo shouldn't matter, but this here bug means it does.


In analogy/joke form:

Patient: Doctor, it hurts every time I lift up my arm!
Doctor: Well don't do that!

Not helpful, doc.

@bep
Collaborator
bep commented Jun 13, 2016

The port I choose for hugo shouldn't matter, but this here bug means it does.

If you could provide a reason other than convenience, the issue might get higher priority, that's all.

@anthonyfok
Collaborator

This piqued my interest, and I decided to try it with Hugo v0.16 (hugo_0.16-1_armhf.deb) running on my Raspberry Pi 3:

sudo hugo server --bind=0.0.0.0 --baseURL=http://rpi3:80/ --port=80 \
                 --watch=true --theme=shiori --disableLiveReload=false

(I set my home router resolves rpi3 to the IP address of the Ethernet port.) The symptoms are as @chuyskywalker described, i.e. LiveReload not working, and the Chrome console (opened by Ctrl-Shift-J on my notebook running Debian sid (amd64)) shows

WebSocket connection to 'ws://rpi3:35729/livereload' failed: Error in connection establishment: net::ERR_CONNECTION_REFUSED

So that answered two questions that I was going to ask @chuyskywalker:

  1. Is it reproducible outside of Docker? Yes.
  2. Is it reproducible with Hugo v0.16? Yes.

I then ran hugo server not on Raspberry Pi 3 but on my own notebook (amd64), and got the same error.

That said, I don't think Hugo ever sets LiveReload to listen at 35729 by default. When hugo server is called without setting any port, it defaults to 1313, and opens only that port AFAIK:

$ sudo netstat -lnp | grep hugo
tcp        0      0 127.0.0.1:1313          0.0.0.0:*               LISTEN      4229/hugo

So, my suspicion would be on the client-side livereload.js. But let's google first, and lo and behold!

So, it is Google Chrome and Mozilla Firefox's doing. And it is probably relatively recent too, which explains why it used to work for you, but no longer.

Since it is not Hugo's bug, we can choose to:

  1. Do nothing. (Haha!)
  2. Add a new feature to allow Hugo listen on 35729 port specifically for LiveReload when --port=80 or --port=443 is used.

Probably not too high priority. Any volunteers? 😉

@chuyskywalker

Actually, based on one of your links, livereload can be told what port to use, ala

<script src="https://localhost:443/livereload.js?port=443"></script>
<script src="https://localhost/livereload.js?port=80"></script>

Seems like it would be sane to inject that into the URL based on the port that Hugo is bound to

@chuyskywalker

I have exactly zero go experience, so this is a WILD shot in the dark, and probably wrong in so very many ways, but...

a049d8e

@anthonyfok
Collaborator

Actually, based on one of your links, livereload can be told what port to use, ala...

@chuyskywalker Good catch! I missed or misunderstood that, and glad that you came up with a fix! It looks good to me at first glance, though I haven't tested it yet.

Care to make a pull request so more experienced Hugo developers can look at it and eventually merge it? See our https://github.com/spf13/hugo/blob/master/CONTRIBUTING.md guide. Many thanks!

@chuyskywalker

Actually, my code fails. I'm not a go dev and I only have the docker build step to use for testing it -- figured I might get lucky but it didn't work out as expected. I'm pretty sure the logic is something like the above, but I likely won't be the one to fix it.

@dplesca dplesca added a commit to dplesca/hugo that referenced this issue Jun 17, 2016
@dplesca dplesca transform: Explicitly bind livereload to server port
Fixes #2205
8323f69
@chuyskywalker

@dplesca -- I copied dplesca@8323f69 into my own repo, compiled, and tried it. Tests pass and live reload works on port 80 afterwards, yay!

@chuyskywalker chuyskywalker added a commit to chuyskywalker/hugo that referenced this issue Jun 24, 2016
@chuyskywalker chuyskywalker Bind livereload to the same port as hugo
If hugo is run with port 80 or 443, live reload does not
correctly bind to the same port, instead using port 35729.
This commit adds functionality to inform livereload of the
correct port to bind to.

Fixes #2205
31fea04
@anthonyfok anthonyfok added a commit that closed this issue Jun 30, 2016
@dplesca @anthonyfok dplesca + anthonyfok transform: Explicitly bind LiveReload to server port
If hugo server is run on port 80 or 443, LiveReload does not
correctly bind to the same port, instead using port 35729.
This commit adds functionality to inform LiveReload of the
correct port to bind to.

See livereload/livereload-js#16

Partially contributed by Jeff Minard (@chuyskywalker).

Fixes #2205
7e08d23
@anthonyfok anthonyfok added the kind/bug label Jun 30, 2016
@anthonyfok anthonyfok added this to the v0.17 milestone Jun 30, 2016
@anthonyfok
Collaborator

Thank you @chuyskywalker and @dplesca for your contribution!
Your group effort has been merged as 7e08d23.

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