yawaramin / re-web

Experimental web framework for ReasonML & OCaml
https://yawaramin.github.io/re-web/re-web/index.html
MIT License
261 stars 8 forks source link

Websockets are broken #28

Closed tcoopman closed 4 years ago

tcoopman commented 4 years ago

So I've tried to fix the fullstack-reason for the latest re-web (https://github.com/yawaramin/fullstack-reason/pull/4) but when I run the frontend/backend I get a 500 on the websocket.

The server seems to receive the incoming request, but it never gets into the socket method. The server also doesn't print any errors...

I've tried the same upgrade on my own code and I have the same issue.

yawaramin commented 4 years ago

My WebSocket examples are working though:

(Terminal 1)
$ esy bin
ReWeb.Server: listening on port 8080

(Terminal 2)
$ websocat ws://127.0.0.1:8080/ticks
{"data": {"tick": "Hello, World!"}}
{"data": {"tick": "Hello, World!"}}
...

This is likely not an issue in ReWeb itself, but I'll investigate. Btw I've pushed a ReWeb upgrade to fullstack-reason; I'll try to reproduce your issue on top of that using the PR you sent.

tcoopman commented 4 years ago

Hmm, what you are doing doesn't seem to work for me.

> cd re-web
> git pull
> esy bin

In firefox:

new WebSocket("ws://localhost:8080/ws");
WebSocket { url: "ws://localhost:8080/ws", readyState: 0, bufferedAmount: 0, onopen: null, onerror: null, onclose: null, extensions: "", protocol: "", onmessage: null, binaryType: "blob" }
>> Firefox can’t establish a connection to the server at ws://localhost:8080/ws.

exactly the same thing with /ticks

In the network tab I get a 500 on both urls

tcoopman commented 4 years ago

Ok, so I found something interesting...

I've tried the same code in Chrome and there it seems to work. So it's not working in firefox, but it is working in chrome

tcoopman commented 4 years ago

I can confirm that this is an upstream issue. See the issue I've created on websocketaf.

anmonteiro commented 4 years ago

I fixed this in https://github.com/anmonteiro/websocketaf/pull/16.

Also note that this should probably not have been a 5XX on Re-Web's side, because websocketaf deemed it as a client error.