Skip to content

Commit

Permalink
Remove uneeded heartbeat since cowboy handles timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
chrismccord committed Oct 20, 2015
1 parent 234995e commit 7b252f4
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 38 deletions.
30 changes: 2 additions & 28 deletions lib/phoenix/transports/websocket.ex
Expand Up @@ -24,8 +24,6 @@ defmodule Phoenix.Transports.WebSocket do
It may be set to `false` (not recommended) or to a list of explicitly It may be set to `false` (not recommended) or to a list of explicitly
allowed origins allowed origins
* `:heartbeat` - the heartbeat interval in milliseconds, default `30_000`
* `:code_reloader` - optionally override the default `:code_reloader` value * `:code_reloader` - optionally override the default `:code_reloader` value
from the socket's endpoint from the socket's endpoint
Expand All @@ -44,9 +42,8 @@ defmodule Phoenix.Transports.WebSocket do


def default_config() do def default_config() do
[serializer: Phoenix.Transports.WebSocketSerializer, [serializer: Phoenix.Transports.WebSocketSerializer,
timeout: :infinity, timeout: 60_000,
transport_log: false, transport_log: false]
heartbeat: 30_000]
end end


def handlers() do def handlers() do
Expand All @@ -56,7 +53,6 @@ defmodule Phoenix.Transports.WebSocket do
## Callbacks ## Callbacks


import Plug.Conn, only: [fetch_query_params: 1, send_resp: 3] import Plug.Conn, only: [fetch_query_params: 1, send_resp: 3]
import Phoenix.Utils, only: [now_ms: 0]


alias Phoenix.Socket.Broadcast alias Phoenix.Socket.Broadcast
alias Phoenix.Socket.Transport alias Phoenix.Socket.Transport
Expand Down Expand Up @@ -100,24 +96,19 @@ defmodule Phoenix.Transports.WebSocket do
Process.flag(:trap_exit, true) Process.flag(:trap_exit, true)
serializer = Keyword.fetch!(config, :serializer) serializer = Keyword.fetch!(config, :serializer)
timeout = Keyword.fetch!(config, :timeout) timeout = Keyword.fetch!(config, :timeout)
heartbeat = Keyword.fetch!(config, :heartbeat)


if socket.id, do: socket.endpoint.subscribe(self, socket.id, link: true) if socket.id, do: socket.endpoint.subscribe(self, socket.id, link: true)


Process.send_after(self, :phoenix_heartbeat, heartbeat)


{:ok, %{socket: socket, {:ok, %{socket: socket,
channels: HashDict.new, channels: HashDict.new,
channels_inverse: HashDict.new, channels_inverse: HashDict.new,
client_last_active: now_ms(),
heartbeat_interval: heartbeat,
serializer: serializer}, timeout} serializer: serializer}, timeout}
end end


@doc false @doc false
def ws_handle(opcode, payload, state) do def ws_handle(opcode, payload, state) do
msg = state.serializer.decode!(payload, opcode: opcode) msg = state.serializer.decode!(payload, opcode: opcode)
state = bump_client_last_active(state)


case Transport.dispatch(msg, state.channels, state.socket) do case Transport.dispatch(msg, state.channels, state.socket) do
:noreply -> :noreply ->
Expand Down Expand Up @@ -150,23 +141,10 @@ defmodule Phoenix.Transports.WebSocket do
format_reply(msg, state) format_reply(msg, state)
end end


def ws_info(:phoenix_heartbeat, state) do
if client_unresponsive?(state) do
{:shutdown, state}
else
Process.send_after(self, :phoenix_heartbeat, state.heartbeat_interval)
encode_reply Transport.heartbeat_message(), state
end
end

def ws_info(_, state) do def ws_info(_, state) do
{:ok, state} {:ok, state}
end end


defp client_unresponsive?(state) do
now_ms() - state.client_last_active > (state.heartbeat_interval * 2)
end

@doc false @doc false
def ws_terminate(_reason, _state) do def ws_terminate(_reason, _state) do
:ok :ok
Expand Down Expand Up @@ -197,10 +175,6 @@ defmodule Phoenix.Transports.WebSocket do
{:reply, {encoding, encoded_payload}, state} {:reply, {encoding, encoded_payload}, state}
end end


defp bump_client_last_active(state) do
%{state | client_last_active: now_ms()}
end

defp code_reload(conn, opts, endpoint) do defp code_reload(conn, opts, endpoint) do
reload? = Keyword.get(opts, :code_reloader, endpoint.config(:code_reloader)) reload? = Keyword.get(opts, :code_reloader, endpoint.config(:code_reloader))
if reload?, do: Phoenix.CodeReloader.reload!(endpoint) if reload?, do: Phoenix.CodeReloader.reload!(endpoint)
Expand Down
11 changes: 2 additions & 9 deletions test/phoenix/integration/websocket_test.exs
Expand Up @@ -63,7 +63,7 @@ defmodule Phoenix.Integration.WebSocketTest do
channel "rooms:*", RoomChannel channel "rooms:*", RoomChannel


transport :websocket, Phoenix.Transports.WebSocket, transport :websocket, Phoenix.Transports.WebSocket,
check_origin: ["//example.com"], heartbeat: 100 check_origin: ["//example.com"], timeout: 100


def connect(%{"reject" => "true"}, _socket) do def connect(%{"reject" => "true"}, _socket) do
:error :error
Expand Down Expand Up @@ -238,21 +238,14 @@ defmodule Phoenix.Integration.WebSocketTest do
assert log =~ "The client's requested channel transport version \"123.1.1\" does not match server's version" assert log =~ "The client's requested channel transport version \"123.1.1\" does not match server's version"
end end


test "sends heartbeat and shuts down if client goes quiet" do test "shuts down if client goes quiet" do
{:ok, socket} = WebsocketClient.start_link(self, "ws://127.0.0.1:#{@port}/ws/websocket") {:ok, socket} = WebsocketClient.start_link(self, "ws://127.0.0.1:#{@port}/ws/websocket")
Process.monitor(socket) Process.monitor(socket)
WebsocketClient.send_heartbeat(socket) WebsocketClient.send_heartbeat(socket)
assert_receive %Message{event: "phx_reply", assert_receive %Message{event: "phx_reply",
payload: %{"response" => %{}, "status" => "ok"}, payload: %{"response" => %{}, "status" => "ok"},
ref: "1", topic: "phoenix"} ref: "1", topic: "phoenix"}


assert_receive %Message{event: "heartbeat", topic: "phoenix", payload: %{}}, 200

WebsocketClient.send_heartbeat(socket)
assert_receive %Message{event: "phx_reply",
payload: %{"response" => %{}, "status" => "ok"},
ref: "2", topic: "phoenix"}, 200

assert_receive {:DOWN, _, :process, ^socket, :normal}, 400 assert_receive {:DOWN, _, :process, ^socket, :normal}, 400
end end
end end
2 changes: 1 addition & 1 deletion test/phoenix/socket_test.exs
Expand Up @@ -66,7 +66,7 @@ defmodule Phoenix.SocketTest do


test "transport config is exposted and merged with prior registrations" do test "transport config is exposted and merged with prior registrations" do
ws = {Phoenix.Transports.WebSocket, ws = {Phoenix.Transports.WebSocket,
[timeout: 1234, serializer: Phoenix.Transports.WebSocketSerializer, transport_log: false, heartbeat: 30_000]} [timeout: 1234, serializer: Phoenix.Transports.WebSocketSerializer, transport_log: false]}


lp = {Phoenix.Transports.LongPoll, lp = {Phoenix.Transports.LongPoll,
[window_ms: 10000, pubsub_timeout_ms: 2000, serializer: Phoenix.Transports.LongPollSerializer, [window_ms: 10000, pubsub_timeout_ms: 2000, serializer: Phoenix.Transports.LongPollSerializer,
Expand Down

0 comments on commit 7b252f4

Please sign in to comment.