Websockets #17

Closed
wants to merge 2 commits into
from

6 participants

@jeregrine
phoenixframework member

Example

websocket "/my_ws_route", WsHandler

TODO

Raw Websockets
  • Websocket Macro
  • Websocket Macro Example
  • Websocket Macro Test
  • Websocket Basic Tests
  • Update Docs
  • Integration Tests
@jeregrine
phoenixframework member

What I want to do next is to define a "WebsocketController" to abstract away the cowboy parts, similar to use GenServer.Behaviour that defines the basic callbacks for the handler and let the user override them.

I think I am going to start implementing SockJS and rely on something like https://github.com/sockjs/sockjs-erlang/tree/master/examples/multiplex to do the multiplexing. Should be super easy to setup in this PR.

I also need to add better documentation. I can also remove the static stuff, not required.

@guilleiguaran guilleiguaran commented on an outdated diff Feb 3, 2014
lib/phoenix/controller/channel.ex
+ Below is an example of an EcoServer websocket.
+ defmodule Router do
+ use Phoenix.Router
+ channel "echo", Channel
+ end
+
+ defmodule Websocket do
+ use Phoenix.Controller.Channel
+ def recieve(conn, data, state) do
+ conn.send(data)
+ end
+ end
+
+ There are 4 callbacks you can override to handle the common functions of a websocket.
+ init(conn, state)
+ recieve(conn, data, state)
@guilleiguaran
guilleiguaran added a line comment Feb 3, 2014

typo in recieve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@knewter knewter commented on an outdated diff Feb 5, 2014
lib/phoenix/controller/channel.ex
@@ -0,0 +1,111 @@
+defmodule Phoenix.Controller.Channel do
+ @moduledoc """
+ This module is a convenience for setting up a basic sockjs websocket.
+
+ ## Example
+
+ Below is an example of an EcoServer websocket.
@knewter
knewter added a line comment Feb 5, 2014

Typo in Eco (that's what we're doing right?) 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@knewter knewter commented on an outdated diff Feb 5, 2014
lib/phoenix/controller/channel.ex
+ """
+ def info(conn, info, state) do
+ {:ok, state}
+ end
+
+ @doc false
+ def __service(conn, {:info, info}, state) do
+ info(conn, info, state)
+ end
+
+ @doc """
+ Callback to handle broadcasts from the other clients on this channel
+ Default implementation simply passes the data to the client.
+ """
+ def broadcast(conn, data, state) do
+ conn.send(data)
@knewter
knewter added a line comment Feb 5, 2014

strange indentation in some of these methods fwiw...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@knewter knewter commented on an outdated diff Feb 5, 2014
lib/phoenix/controller/websocket.ex
@@ -0,0 +1,83 @@
+defmodule Phoenix.Controller.Websocket do
+ @moduledoc """
+ This module is a convenience for setting up a basic sockjs websocket.
+
+ ## Example
+
+ Below is an example of an EcoServer websocket.
@knewter
knewter added a line comment Feb 5, 2014

Echo...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeregrine
phoenixframework member

Sorry for flaking out on this. We've been moving/painting both places and I've been exhausted as a result. I am going to split this into a websocket portion and a channel portion worry about channels later, as they add signifigant complexity.

Also might make sense to move much of this into an adapter specific plug? And then the channel implementation could just use it?

@chrismccord
phoenixframework member

No worries. I don't think we can leverage a Plug middleware in this case since it's not conn dependent, and we'll need to passing options directly to the Plug.Adapters.Cowboy.http adapter. I think something along the lines of your initial approach is the direction we'll need to go.

@jeregrine
phoenixframework member
@chrismccord chrismccord and 1 other commented on an outdated diff Feb 22, 2014
lib/phoenix/controller/websocket.ex
+ end
+
+ There are 4 callbacks you can override to handle the common functions of a websocket.
+ start(conn, state)
+ receive(data, conn, state)
+ info(data, conn, state)
+ closed(data, conn, state)
+
+ Each function is an alias for the websocket handler functions, for more information on connections and
+ what you should return from these functions check out the websocket documentation.
+
+
+
+ """
+ defmacro __using__(state \\ []) do
+ quote do
@chrismccord
phoenixframework member
chrismccord added a line comment Feb 22, 2014

We can import most of these funcs. The goal is to do as little code generation as possible.

@jeregrine
phoenixframework member
jeregrine added a line comment Feb 22, 2014

I was basing it how jose did gen_server behavior https://github.com/elixir-lang/elixir/blob/master/lib/elixir/lib/gen_server/behaviour.ex

init, websocket_* need to be in the module definition or else cowboy doesn't see them. And the others are overrideable.

@chrismccord
phoenixframework member
chrismccord added a line comment Feb 22, 2014

Ah, yes you're completely right. Carry on 👍

@jeregrine
phoenixframework member
jeregrine added a line comment Feb 22, 2014

Thanks for the feedback though! I'm fairly new to macros :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeregrine
phoenixframework member

Here is an example for everyone interested https://gist.github.com/jeregrine/af7ecb4c7d87767411a0

@jeregrine jeregrine commented on an outdated diff Feb 22, 2014
lib/phoenix/controller/websocket.ex
+
+ Below is an example of an EchoServer websocket.
+ defmodule Router do
+ use Phoenix.Router
+ websocket "echo", Websocket
+ end
+ defmodule Websocket do
+ use Phoenix.Controller.Websocket
+ def receive(conn, data, state) do
+ conn.send(data)
+ end
+ end
+
+ There are 4 callbacks you can override to handle the common functions of a websocket.
+ start(conn, state)
+ receive(data, conn, state)
@jeregrine
phoenixframework member
jeregrine added a line comment Feb 22, 2014

I should probably rename this to something else as to not conflict with the "receive" keyword.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeregrine
phoenixframework member

I updated the docs heavily and changed receive => stream. Last step is to test everything.

My Approach is:

And from there I will need direction. I think I could test that the default implementations return what I say they do? I'm not completely sure HOW to integration test this without an external websocket client lib.

@jeregrine
phoenixframework member

Also thinking it might be nice to have functions like

MySocketController.send(process, "data") 

which basically wraps a send to handle info.

@knewter
@chrismccord chrismccord commented on an outdated diff Feb 22, 2014
lib/phoenix/router/router.ex
@@ -26,8 +28,12 @@ defmodule Phoenix.Router do
end
def start do
- IO.puts ">> Running #{__MODULE__} with Cowboy with #{inspect @options}"
- Plug.Adapters.Cowboy.http __MODULE__, [], @options
+ if dispatch = Phoenix.Router.setup_cowboy_dispatch(__MODULE__, @options, @dispatch_options) do
@chrismccord
phoenixframework member
chrismccord added a line comment Feb 22, 2014

We need to abstract dispatch_options handling and move it out of the Router. Perhaps Phoenix.Adapters.Cowboy.merge_dispatch_options/3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chrismccord chrismccord and 1 other commented on an outdated diff Feb 22, 2014
lib/phoenix/router/router.ex
+ defmacro websocket(path, handler, options \\ []) do
@chrismccord
phoenixframework member
chrismccord added a line comment Feb 22, 2014

Let's move this to Phoenix.Router.WebsocketMapper

The Router's __using__ block can just use Phoenix.Router.WebsocketMapper similar to the http mapper.

@jeregrine
phoenixframework member
jeregrine added a line comment Feb 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jeregrine
phoenixframework member

@knewter could maybe co-opt something like this https://github.com/meh/elixir-socket

@jeregrine
phoenixframework member

@chrismccord I made those changes. next step is refactoring per that email. Should be a simple step.

@jeregrine
phoenixframework member

@chrismccord convenience functions added

@jeregrine
phoenixframework member

I refactored the helper functions also updated the original gist for those following along. https://gist.github.com/jeregrine/af7ecb4c7d87767411a0

@jeregrine
phoenixframework member

@chrismccord I did a straight brain dump on the protocol and the pros/cons of bullet vs websocket.js.

Making the ping/pong is easy enough and it only serves as to give the user and browser a sense of "hey pongs/frames are not coming back or coming back slowly maybe the server is disconnected?"
Example: I watch slack's ping/pings and they will send almost 4 pings before the "onClose" method is called on the websocket. Then slack implements a retry connection with a backoff with each new failure and allows users to click reconnect.

So I am leaning towards it websocket-js.

@jeregrine
phoenixframework member

@HashNuke had a good point and maybe we should build it around a system like Faye http://faye.jcoglan.com/

@jeregrine
phoenixframework member

Thats probably an entirely different project and a TON of work though. But we will probably be reinventing the wheel if we don't use something like it. :(

@patrickdet

have you guys decided how to proceed?

@chrismccord
phoenixframework member

I'm going to start prototyping out a websocket/channel implementation. We've discussed implementing parts of the faye protocol, but how far we head in that direction will largely depend on initial results as we're speccing things out.

@chrismccord
phoenixframework member

We should have a "raw" websocket handler in place soon for those just wanting the bare bones handlers, but our focus will be on a higher level system for managing connections, join/leave, and channel support.

@jeregrine
phoenixframework member
@tecnobrat

Dropping websocket support?

@HashNuke

@tecnobrat I think it's more like... coming soon (like in a few hours or days)

@jeregrine
phoenixframework member
@chrismccord
phoenixframework member

@tecnobrat No way! We're landing the websocket/channel layer in master today! Check out the recent discussions/feature branch
#70

@HashNuke

@tecnobrat See I told you :D
@chrismccord and @jeregrine Congrats ~! awesome work 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment