Azolo / websockex

An Elixir Websocket Client
MIT License
515 stars 97 forks source link

Can't send sync call within handle_* methods #34

Closed Grafikart closed 6 years ago

Grafikart commented 7 years ago

Problem

I wanted to send a message when I connect using websockets. Using your EchoClient example I added this callback

  def handle_connect(conn, state) do
    pid = self()
    EchoClient.echo(pid, "Hello")
    {:ok, state}
  end

Sending message: Hello is logged but nothing is received. If I do

  def handle_connect(conn, state) do
    pid = self()
    spawn fn -> EchoClient.echo(pid, "Hello") end
    {:ok, state}
  end

Everything works as expected, the message is sent and received. I don't understand why I needed to do the spawn ? Is this an issue with the library or with my understanding of processes ?

Azolo commented 7 years ago

It's an issue with documentation.

send_frame is a synchronous rpc to a WebSockex process. Meaning it has the same semantics as trying to have a GenServer use call/2 on itself in a callback.

This is because of the disconnecting semantics. It can't go through the disconnect flow in the middle of a callback.

If there is something that you need that you can't handle with the {:reply, frame} return please let me know so I can think of a solution.

Otherwise, for this I will add better documentation and make it raise an error next release.

edennis commented 7 years ago

I'm running into the same issues. I used to have a single process that was responsible for reading from and writing to a websocket. With this change it's no longer possible to write to the websocket from within the same process. I was using this for two things:

Here is roughly what I'm doing:

defmodule WS do
  use WebSockex

  def start_link(host, subscription) do
    opts = [handle_initial_conn_failure: true, async: true]
    {:ok, pid} = WebSockex.start_link(host, __MODULE__, %{subscription: subscription}, opts)
    schedule_keep_alive(pid)
    {:ok, pid}
  end

  def handle_connect(_conn, state) do
    subscribe(state.subscription)
    {:ok, state}
  end

  def handle_frame({:text, msg}, state) do
    # stuff...
    {:ok, state}
  end

  def handle_disconnect({:local, reason}, state) do
    # stuff...
    {:reconnect, state}
  end

  def handle_info(:keep_alive, state) do
    ping()
    schedule_keep_alive()
    {:ok, state}
  end

  defp schedule_keep_alive(pid \\ self()) do
    Process.send_after(pid, :keep_alive, 15_000)
  end

  defp ping, do: subscribe()

  defp subscribe(channel \\ "")  do
    msg = Poison.encode!(%{sub: channel})
    WebSockex.send_frame(self(), {:text, msg})
  end
end

As I see it, now if I want to send a message over the websocket I need to do it from a different process. That doesn't seem right if I need to reissue a "subscribe" command if the connection is dropped. I'm also not really satisfied with just spawning another process to avoid the gen_server timeout.

Could you shed some light on what the reason was for this change?

Azolo commented 7 years ago

@edennis Sure.

The main reason was when the connection was closing or had not been established yet and you tried to use send_frame there was no indication that your frame wasn't being sent. Instead of sending message that may be sent, I wanted to provide some feedback that it wasn't.

The other reason was to promote the use of the {:reply, frame, state} callback return syntax. Looking at your code I would suggest doing this:

def handle_info(:keep_alive, %{channel: channel_from_state} = state) do
    schedule_keep_alive()
    msg = {:text, Poison.encode!(%{sub: channel_from_state})}
    {:reply, msg, state}
end

Which really brings us back to my documentation sucks. Sorry

edennis commented 7 years ago

@Azolo thanks for the clarification! I was able to get the upgrade to 0.4.0 working with your pointer. I guess it may be a documentation issue, but if people are like me, they tend to look at the examples first and when they get stuck fallback to the docs. Based on that assumption, I updated your echo client example to include a {:reply, frame, state} callback return: https://github.com/Azolo/websockex/pull/40

masterkain commented 6 years ago

I need to be able to send multiple messages over a ws connection.

https://gist.github.com/prdn/b8c067c758aab7fa3bf715101086b47c#file-bfx_test_book-js-L45

see this code, I send a subscription message in handle_frame when I get a known message, but now I need to send multiple, let alone adding authentication. I'm running in a supervisor, suggestions?

  def handle_frame({type, msg}, state) do
    # %{
    #   "event" => "info",
    #   "platform" => %{"status" => 1},
    #   "serverId" => "baab0c29-eb4c-4ff3-abb8-86eb92fc6ac4",
    #   "version" => 2
    # }

    # %{
    #   "chanId" => 8,
    #   "channel" => "book",
    #   "event" => "subscribed",
    #   "freq" => "F0",
    #   "len" => "25",
    #   "pair" => "BTCUSD",
    #   "prec" => "P0",
    #   "symbol" => "tBTCUSD"
    # }

    decoded_frame = Poison.decode!(msg)

    if Map.has_key?(decoded_frame, "version") do
      msg = Poison.encode!(%{"event" => "subscribe", "channel" => "book", "symbol" => "tBTCUSD"})
      {:reply, {:text, msg}, :fake_state}
    else
      {:ok, state}
    end
  end
Azolo commented 6 years ago

@masterkain That's a good feature request. Could you make a new issue?

In the interim, I suggest WebSockex.cast(self(), {:send_message, frame}) and

def handle_cast({:send_message, frame}, state) do
  {:reply, frame, state}
end

That way, you can queue the messages you want to send in order in the process mailbox, then return {:ok, state} and the messages will fire off as they are dealt with in the mailbox.

ByeongUkChoi commented 1 year ago

At first, I didn't know why I couldn't do self-calling here. But {:reply, msg, state} of handle_info can be used to make a self-call. It's awesome!!