ninenines / gun

HTTP/1.1, HTTP/2, Websocket client (and more) for Erlang/OTP.
ISC License
891 stars 232 forks source link

WebSocket upgrade is process dependent #204

Closed GuillaumeMilan closed 4 years ago

GuillaumeMilan commented 4 years ago

When you upgrade a connection into a websocket, this upgrade is process dependent. As I don't know if this is a bug or intended I open this issue. But feel free to close it if it's the normal behavior.

Description:

I have an erlang process A that create the connection C via: gun:open and upgrade it via the commande gun:ws_upgrade. Then I receive the gun_upgrade message and store the connection PID and the upgrade Reference.

Then I spawn a new process B that receive as parameter a function executing a gun:ws_send/1 on the previous obtained PID and Reference.

And my process B receives the following message: Connection needs to be upgraded to Websocket before the gun:ws_send/1 function can be used.

I am sure it is not a synchronization probleme as I successfully send messages with the process A before creating the process B.

Reproduce:

I am sorry, I am developping in elixir and not in erlang so the code will be in elixir.

defmodule Reception do
  def reception(scope) do
    receive do
      {:gun_ws,^wsconn,^wsref,{:text,msg}} -> IO.puts("received #{msg} \nIn scope #{scope}")
      {:gun_error, ^wsconn,{_reason,msg}} -> IO.puts("ERROR on socket #{msg}\nIn scope #{scope}")
    end
  end
end

url = ''
{:ok,wsconn} = gun_open_wrapper('#{host}', port); {:ok,:http} = :gun.await_up(wsconn)
:gun.ws_upgrade(wsconn,url)
wsref = 
    receive do
      {:gun_upgrade, ^wsconn, ref, ["websocket"], _headers}-> ref
      other -> exit(other)
    after 2_000 -> exit(:ws_upgrade_timeout) end
state = %{wsconn: wsconn, wsref: wsref}
msg = "toto"
:gun.ws_send(wsconn,{:text, msg})
Reception.reception("attached")
Task.await(fn -> 
  :gun.ws_send(wsconn,{:text, msg})
  Reception.reception("detached")
end)
essen commented 4 years ago

It's a bad error message. The intent is for Websocket to be restricted by default to one process, but I suppose an option could be added to remove that behavior. The ws_upgrade should also accept the reply_to option like the other request functions.

GuillaumeMilan commented 4 years ago

So it is not possible to upgrade in one process and send messages in an others process (both process are intended to send messages)?

essen commented 4 years ago

Not yet but I guess that'll make it into 2.0. A quick and dirty way to allow it would be to remove the owner=Owner part of this line: https://github.com/ninenines/gun/blob/master/src/gun.erl#L1037

essen commented 4 years ago

I've added reply_to, removed the notowner entirely and propagated the reply_to to all protocols when upgrading/using CONNECT. I've also added a function to change the owner of a connection as this is still relevant for automatic shutdown and up/down messages. Closing, thanks!