Playground: Fix pass bound hostname to run sessions

[orangeSi]

I test crystal-1.5.0-1 and crystal-1.3.2-1 both got this bug:
I have two servers(A and B, they both are in same local area network by one WIFI):
A: ip 192.168.8.83, Unbuntu 18
B: Windows10
when run crystal play -p 3566 -b 192.168.8.83 -v in server A, then open http://192.168.8.83:3566/ in Chrome at server B then click start button will get this error:

Unhandled exception: Error connecting to 'localhost:3566': Connection refused (Socket::ConnectError)
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/socket/addrinfo.cr:67:17 in 'initialize'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/socket/tcp_socket.cr:27:3 in 'initialize'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/socket/tcp_socket.cr:27:3 in 'new'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/http/web_socket/protocol.cr:275:5 in 'new'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/http/web_socket/protocol.cr:326:14 in 'new'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/http/web_socket.cr:37:5 in 'new'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/http/web_socket.cr:36:3 in 'new'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/compiler/crystal/tools/playground/agent.cr:8:11 in 'initialize'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/compiler/crystal/tools/playground/agent.cr:7:3 in 'new'
  from playground_prelude:5:24 in '~Crystal::Playground::Agent::instance:init'
  from play:1:6 in '__crystal_main'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/crystal/main.cr:115:5 in 'main_user_code'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/crystal/main.cr:101:7 in 'main'
  from /home/myth/crystal/crystal-1.3.2-1/share/crystal/src/crystal/main.cr:127:3 in 'main'
  from /lib/x86_64-linux-gnu/libc.so.6 in '__libc_start_main'
  from /home/myth/.cache/crystal/crystal-run-play-1-1.tmp in '_start'
  from ???

exit status: 1

Update:
when I change @@instance = Crystal::Playground::Agent.new("ws://localhost:#{port}/agent/#{session_key}/#{tag}", #{tag}) of src/compiler/crystal/tools/playground/server.cr to @@instance = Crystal::Playground::Agent.new("ws://192.168.8.83:#{port}/agent/#{session_key}/#{tag}", #{tag}) and rebuild crystal, it works ！

[straight-shoota]

The best way to get a working value is probably to use the actual bind address. We're currently only using the port from that, but it should use the entire address.

https://github.com/orangeSi/crystal/blob/a3a313c7154457114019b9de2ebd748d25220111/src/compiler/crystal/tools/playground/server.cr#L524-L525

[straight-shoota]

This should be an instance variable, not a class variable, right?

[orangeSi]

I first use instance variable @host, but got error:

26 | ip = @host
          ^----
Error: @instance_vars are not yet allowed in metaclasses: use @@class_vars instead

So change to @@host.

[straight-shoota]

That's not a proper solution. The error appears because you tried to access the ivar directly from a class method. That doesn't work because it's not bound to an instance. You should pass the ivar value to the class method, just like @port (or just combine them into @address, which probably makes even more sense).

[orangeSi]

thanks! I had try to modify it by another commit right now!

[straight-shoota]

I don't think this is correct. You can't assume nil or 127.0.0.1 mean localhost.
Also, any kind of modification should happen already at the server, not in each session.

[orangeSi]

I don't think this is correct. You can't assume nil or 127.0.0.1 mean localhost. Also, any kind of modification should happen already at the server, not in each session.

yes, I also don't think is correct , I assume if specify the -b then use ip of -b instead of default localhost. I just try to improve that becuase the raw code is alaways localhost.

[orangeSi]

I don't think this is correct. You can't assume nil or 127.0.0.1 mean localhost. Also, any kind of modification should happen already at the server, not in each session.

sorry，I jgot your idea just now, should modification the ip outside of each session!

[orangeSi]

I don't think this is correct. You can't assume nil or 127.0.0.1 mean localhost. Also, any kind of modification should happen already at the server, not in each session.

I had try to fix it in my second commit~

[orangeSi]

The best way to get a working value is probably to use the actual bind address. We're currently only using the port from that, but it should use the entire address.

https://github.com/orangeSi/crystal/blob/a3a313c7154457114019b9de2ebd748d25220111/src/compiler/crystal/tools/playground/server.cr#L524-L525

yes, currently only use port from that and alaways localhost, so I try to improve it by use localhost and -b of crystal play.

[straight-shoota]

For correct behaviour, you must use the exact address returned from server.bind_tcp. Everything else is guesswork when it comes to named hosts.

[orangeSi]

For correct behaviour, you must use the exact address returned from server.bind_tcp. Everything else is guesswork when it comes to named hosts.

ok, now the last commit is using the exact address returned from server.bind_tcp, please review it, thanks~

[straight-shoota]

Sorry, I didn't mean to start a new server. That's completely unnecessary. The address is already available from the existing call to server.bind_tcp:

crystal/src/compiler/crystal/tools/playground/server.cr

Line 519 in 5de237e

address = server.bind_tcp @host || Socket::IPAddress::LOOPBACK, @port

We're already using the port from that address. Instead, we should store the entire address and use that everywhere in place of host + port.

[orangeSi]

already using the port from that address

ok, now change to use the existsed address ip instead of start a new server in the last commit~

[Sija]

Redundant newlines

[orangeSi]

Linux CI / x86_64-gnu-test (1.0.0, -Dwithout_ffi) (pull_request) Failing after 10s — x86_64-gnu-test (1.0.0, -Dwithout_ffi)

got 1 failing check as above, and the log as below, is the network not working so can't docker pull crystallang/crystal:1.0.0-build ?

Run FLAGS=-Dwithout_ffi bin/ci build
make std_spec clean threads=1 junit_output=.junit/std_spec.xml
  verify_linux_environment
  docker run --rm -t -u [10](https://github.com/crystal-lang/crystal/runs/7821428957?check_suite_focus=true#step:5:11)01 -v /home/runner/work/crystal/crystal:/mnt -v /etc/passwd:/etc/passwd -v /etc/group:/etc/group -w /mnt -e CRYSTAL_CACHE_DIR=/tmp/crystal -e SPEC_SPLIT_DOTS crystallang/crystal:1.0.0-build linux64 /bin/sh -c 'make std_spec clean threads=1 junit_output=.junit/std_spec.xml'
  Unable to find image 'crystallang/crystal:1.0.0-build' locally
  docker: Error response from daemon: Get "https://registry-1.docker.io/v2/": EOF.
  See 'docker run --help'.
  on_os linux docker run --rm -t -u 1001 -v /home/runner/work/crystal/crystal:/mnt -v /etc/passwd:/etc/passwd -v /etc/group:/etc/group -w /mnt -e CRYSTAL_CACHE_DIR=/tmp/crystal -e SPEC_SPLIT_DOTS crystallang/crystal:1.0.0-build linux64 /bin/sh -c 'make std_spec clean threads=1 junit_output=.junit/std_spec.xml' exited with [12](https://github.com/crystal-lang/crystal/runs/7821428957?check_suite_focus=true#step:5:13)5
  Error: Process completed with exit code 1.

[straight-shoota]

There seems to have been some connectivity issues with the docker registry in several CI runs. They are sporadic and unrelated to any code changes, so we just ignore them or try again.

[orangeSi]

There seems to have been some connectivity issues with the docker registry in several CI runs. They are sporadic and unrelated to any code changes, so we just ignore them or try again.

ok，thanks for the explanation because I am not familiar with the github CI runs~