Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve crash handling during WebSock.init/1 calls #20

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/websock_adapter/cowboy_adapter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ if Code.ensure_loaded?(:cowboy_websocket) do
end
end

# If websocket_init (or the handler's init) raises an error, state may still
# be the 3-element tuple that we get at initialization time. Since we have
# not yet initialized the websock at this point, just terminate and let
# Cowboy do its usual logging
def terminate(_reason, _req, {_handler, _process_flags, _state}), do: :ok

defp handle_reply({:ok, state}, handler), do: {:ok, {handler, state}}
defp handle_reply({:push, data, state}, handler), do: {:reply, data, {handler, state}}

Expand Down
8 changes: 4 additions & 4 deletions mix.lock
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
%{
"bandit": {:hex, :bandit, "1.4.0", "fdf9c4b9e3a2d8579540ff90f74f514e5bec25f8cb1c7ede6fddd409509e5b4b", [:mix], [{:hpax, "~> 0.1.1", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "2d068334fe7a4ea17161b875aa112bfa7d62060e8eefb1a1117b2ab6a817e04f"},
"bunt": {:hex, :bunt, "0.2.1", "e2d4792f7bc0ced7583ab54922808919518d0e57ee162901a16a1b6664ef3b14", [:mix], [], "hexpm", "a330bfb4245239787b15005e66ae6845c9cd524a288f0d141c148b02603777a5"},
"bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},
"cowboy": {:hex, :cowboy, "2.9.0", "865dd8b6607e14cf03282e10e934023a1bd8be6f6bacf921a7e2a96d800cd452", [:make, :rebar3], [{:cowlib, "2.11.0", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "2c729f934b4e1aa149aff882f57c6372c15399a20d54f65c8d67bef583021bde"},
"cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"},
"cowlib": {:hex, :cowlib, "2.11.0", "0b9ff9c346629256c42ebe1eeb769a83c6cb771a6ee5960bd110ab0b9b872063", [:make, :rebar3], [], "hexpm", "2b3e9da0b21c4565751a6d4901c20d1b4cc25cbb7fd50d91d2ab6dd287bc86a9"},
"credo": {:hex, :credo, "1.6.7", "323f5734350fd23a456f2688b9430e7d517afb313fbd38671b8a4449798a7854", [:mix], [{:bunt, "~> 0.2.1", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "41e110bfb007f7eda7f897c10bf019ceab9a0b269ce79f015d54b0dcf4fc7dd3"},
"credo": {:hex, :credo, "1.7.8", "9722ba1681e973025908d542ec3d95db5f9c549251ba5b028e251ad8c24ab8c5", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cb9e87cc64f152f3ed1c6e325e7b894dea8f5ef2e41123bd864e3cd5ceb44968"},
"dialyxir": {:hex, :dialyxir, "1.3.0", "fd1672f0922b7648ff9ce7b1b26fcf0ef56dda964a459892ad15f6b4410b5284", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "00b2a4bcd6aa8db9dcb0b38c1225b7277dca9bc370b6438715667071a304696f"},
"earmark_parser": {:hex, :earmark_parser, "1.4.29", "149d50dcb3a93d9f3d6f3ecf18c918fb5a2d3c001b5d3305c926cddfbd33355b", [:mix], [], "hexpm", "4902af1b3eb139016aed210888748db8070b8125c2342ce3dcae4f38dcc63503"},
"erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
"ex_doc": {:hex, :ex_doc, "0.29.0", "4a1cb903ce746aceef9c1f9ae8a6c12b742a5461e6959b9d3b24d813ffbea146", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "f096adb8bbca677d35d278223361c7792d496b3fc0d0224c9d4bc2f651af5db1"},
"file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},
"file_system": {:hex, :file_system, "1.0.1", "79e8ceaddb0416f8b8cd02a0127bdbababe7bf4a23d2a395b983c1f8b3f73edd", [:mix], [], "hexpm", "4414d1f38863ddf9120720cd976fce5bdde8e91d8283353f0e31850fa89feb9e"},
"hpax": {:hex, :hpax, "0.1.2", "09a75600d9d8bbd064cdd741f21fc06fc1f4cf3d0fcc335e5aa19be1a7235c84", [:mix], [], "hexpm", "2c87843d5a23f5f16748ebe77969880e29809580efdaccd615cd3bed628a8c13"},
"jason": {:hex, :jason, "1.4.0", "e855647bc964a44e2f67df589ccf49105ae039d4179db7f6271dfd3843dc27e6", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "79a3791085b2a0f743ca04cec0f7be26443738779d09302e01318f97bdb82121"},
"jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
"makeup": {:hex, :makeup, "1.1.0", "6b67c8bc2882a6b6a445859952a602afc1a41c2e08379ca057c0f525366fc3ca", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "0a45ed501f4a8897f580eabf99a2e5234ea3e75a4373c8a52824f6e873be57a6"},
"makeup_elixir": {:hex, :makeup_elixir, "0.16.1", "cc9e3ca312f1cfeccc572b37a09980287e243648108384b97ff2b76e505c3555", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "e127a341ad1b209bd80f7bd1620a15693a9908ed780c3b763bccf7d200c767c6"},
"makeup_erlang": {:hex, :makeup_erlang, "0.1.1", "3fcb7f09eb9d98dc4d208f49cc955a34218fc41ff6b84df7c75b3e6e533cc65f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "174d0809e98a4ef0b3309256cbf97101c6ec01c4ab0b23e926a9e17df2077cbb"},
Expand Down
102 changes: 102 additions & 0 deletions test/websock_adapter/cowboy_adapter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,28 @@ defmodule WebSockAdapterCowboyAdapterTest do
assert recv_connection_close_frame(client) == {:ok, <<5555::16, "BOOM"::binary>>}
assert connection_closed_for_reading?(client)
end

defmodule InitErrorWebSock do
use NoopWebSock

def init(_opts), do: raise("boom")
end

test "crashes are handled in a manner that Cowboy expects", context do
client = tcp_client(context)

warnings =
capture_log(fn ->
http1_handshake(client, InitErrorWebSock)

assert recv_connection_close_frame(client) == {:ok, <<1011::16>>}

# Give our adapter a chance to explode if it's going to
Process.sleep(100)
end)

assert warnings =~ "%RuntimeError{message: \"boom\"}"
end
end

describe "handle_in" do
Expand Down Expand Up @@ -523,6 +545,30 @@ defmodule WebSockAdapterCowboyAdapterTest do
assert recv_connection_close_frame(client) == {:ok, <<5555::16, "BOOM"::binary>>}
assert connection_closed_for_reading?(client)
end

defmodule HandleInErrorWebSock do
use NoopWebSock

def handle_in(_data, _state), do: raise("boom")
end

test "crashes are handled in a manner that Cowboy expects", context do
client = tcp_client(context)

http1_handshake(client, HandleInErrorWebSock)

warnings =
capture_log(fn ->
send_text_frame(client, "OK")

assert recv_connection_close_frame(client) == {:ok, <<1011::16>>}

# Give our adapter a chance to explode if it's going to
Process.sleep(100)
end)

assert warnings =~ "%RuntimeError{message: \"boom\"}"
end
end

describe "handle_control" do
Expand Down Expand Up @@ -825,6 +871,31 @@ defmodule WebSockAdapterCowboyAdapterTest do
assert recv_connection_close_frame(client) == {:ok, <<5555::16, "BOOM"::binary>>}
assert connection_closed_for_reading?(client)
end

defmodule HandleControlErrorWebSock do
use NoopWebSock

def handle_control(_data, _state), do: raise("boom")
end

test "crashes are handled in a manner that Cowboy expects", context do
client = tcp_client(context)

http1_handshake(client, HandleControlErrorWebSock)

warnings =
capture_log(fn ->
send_ping_frame(client, "OK")
_ = recv_pong_frame(client)

assert recv_connection_close_frame(client) == {:ok, <<1011::16>>}

# Give our adapter a chance to explode if it's going to
Process.sleep(100)
end)

assert warnings =~ "%RuntimeError{message: \"boom\"}"
end
end

describe "handle_info" do
Expand Down Expand Up @@ -1102,6 +1173,37 @@ defmodule WebSockAdapterCowboyAdapterTest do
assert recv_connection_close_frame(client) == {:ok, <<5555::16, "BOOM"::binary>>}
assert connection_closed_for_reading?(client)
end

defmodule HandleInfoErrorWebSock do
use NoopWebSock

def handle_in(_data, state), do: {:push, {:text, :erlang.pid_to_list(self())}, state}

def handle_info(_data, _state),
do: raise("boom")
end

test "crashes are handled in a manner that Cowboy expects", context do
client = tcp_client(context)

http1_handshake(client, HandleInfoErrorWebSock)

send_text_frame(client, "whoami")
{:ok, pid} = recv_text_frame(client)
pid = pid |> String.to_charlist() |> :erlang.list_to_pid()

warnings =
capture_log(fn ->
Process.send(pid, "OK", [])

assert recv_connection_close_frame(client) == {:ok, <<1011::16>>}

# Give our adapter a chance to explode if it's going to
Process.sleep(100)
end)

assert warnings =~ "%RuntimeError{message: \"boom\"}"
end
end

describe "terminate" do
Expand Down