Fix Transport heartbeat storing control sockets. Closes phoenixframework#562

chrismccord · chrismccord · commit c6270f68dbbb · 2014-12-19T06:53:37.000-05:00
Control sockets for heartbeat messages were being tracked by
the transport, causing leave events to be dispatched to nowhere.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,8 @@
 ## v0.8.0-dev
 * Enhancements
   * [Router] `protect_from_forgery` has been added to the Router for CSRF protection. This is automatically plugged in new projects. See [this example](https://github.com/phoenixframework/phoenix/blob/ce5ebf3d9de4412a18e6325cd0d34e1b9699fb5a/priv/template/web/router.ex#L7) for plugging in your existing router pipeline(s).
+* Bug Fixes
+  * [Channel] Fix WebSocket Heartbeat causing unnecessary `%Phoenix.Socket{}`'s to be tracked and leave errors on disconnect.
 * Backwards incompatible changes
   * Endpoints should now be explicitly started in your application supervision tree. Just add `worker(YourApp.Endpoint, [])` to your supervision tree in `lib/your_app.ex`
   * `mix phoenix.start` was renamed to `mix phoenix.server`
diff --git a/lib/phoenix/channel/transport.ex b/lib/phoenix/channel/transport.ex
@@ -68,6 +68,8 @@ defmodule Phoenix.Channel.Transport do
     |> case do
       {:ok, socket} ->
         {:ok, HashDict.put(sockets, {msg.channel, msg.topic}, socket)}
+      {:heartbeat, _socket} ->
+        {:ok, sockets}
       {:error, _socket, reason} ->
         {:error, sockets, reason}
     end
@@ -89,7 +91,7 @@ defmodule Phoenix.Channel.Transport do
     msg = %Message{channel: "phoenix", topic: "conn", event: "heartbeat", message: %{}}
     send socket.pid, msg
 
-    {:ok, socket}
+    {:heartbeat, socket}
   end
   def dispatch(socket, channel, "join", msg) do
     socket
diff --git a/test/phoenix/channel_test.exs b/test/phoenix/channel_test.exs
@@ -216,12 +216,13 @@ defmodule Phoenix.Channel.ChannelTest do
     end
   end
 
-  test "phoenix channel returns heartbeat message when received" do
-    sockets = HashDict.put(HashDict.new, {"phoenix", "conn"}, new_socket)
+  test "returns heartbeat message when received, and does not store socket" do
+    sockets = HashDict.new
     message = %Message{channel: "phoenix", topic: "conn", event: "heartbeat", message: %{}}
 
-    assert match?({:ok, _sockets}, Transport.dispatch(message, sockets, self, Router))
+    assert {:ok, sockets} = Transport.dispatch(message, sockets, self, Router)
     assert_received %Message{channel: "phoenix", topic: "conn", event: "heartbeat", message: %{}}
+    assert sockets == HashDict.new
   end
 
   test "socket state can change when receiving regular process messages" do
@@ -242,4 +243,6 @@ defmodule Phoenix.Channel.ChannelTest do
     _socket = MyChannel.event(socket, "get", %{"key" => :val})
     assert_received 123
   end
+
+
 end
