cloudamqp / websocket-tcp-relay

Expose any TCP server as a WebSocket endpoint
Apache License 2.0
30 stars 11 forks source link

Path prefix option #14

Closed carlhoerberg closed 8 months ago

carlhoerberg commented 8 months ago

Fixes #11

dentarg commented 8 months ago

LGTM 👍 Some feedback:

What about making the error be nicer (less scary, crash looking)?

$ bin/websocket-tcp-relay --upstream localhost:15672 --webroot $HOME/Downloads/webroot --prefix=/bar
Unhandled exception: Prefix has to start and end with / (Exception)
  from src/websocket-tcp-relay/prefix_handler.cr:7:5 in 'initialize'
  from src/websocket-tcp-relay/prefix_handler.cr:6:3 in 'new'
  from src/websocket-tcp-relay.cr:86:9 in 'run'
  from src/websocket-tcp-relay.cr:117:1 in '__crystal_main'
  from /opt/homebrew/Cellar/crystal/1.11.2/share/crystal/src/crystal/main.cr:129:5 in 'main_user_code'
  from /opt/homebrew/Cellar/crystal/1.11.2/share/crystal/src/crystal/main.cr:115:7 in 'main'
  from /opt/homebrew/Cellar/crystal/1.11.2/share/crystal/src/crystal/main.cr:141:3 in 'main'

Could also make it more user friendly with not requiring the trailing /? (Just assume one?)

Not sure if important or if anyone care, but as I noticed it, I'll document it here, before, 404 responses always included a body 404 Not Found, that no longer happens when the request doesn't match the prefix

$ curl -s localhost:15670/bar/foo
404 Not Found

$ curl -s -v localhost:15670/foo
*   Trying [::1]:15670...
* Connected to localhost (::1) port 15670
> GET /foo HTTP/1.1
> Host: localhost:15670
> User-Agent: curl/8.4.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Connection: keep-alive
< Content-Length: 0
<
* Connection #0 to host localhost left intact