phoenixframework / websock_adapter

Implementation of the WebSock specification for servers
MIT License
32 stars 10 forks source link

Missing function clause in terminate callback (cowboy adapter) #19

Open focused opened 3 weeks ago

focused commented 3 weeks ago

When I got an error the terminate callback can not handle it correctly:

"** (FunctionClauseError) no function clause matching in WebSockAdapter.CowboyAdapter.terminate/3\n    (websock_adapter 0.5.7) lib/websock_adapter/cowboy_adapter.ex:52: 

WebSockAdapter.CowboyAdapter.terminate({:crash, :error, :function_clause}, %{port: 443, scheme: \"https\", version: :\"HTTP/1.1\", path: \"*****\", host: \"*****\", peer: {{*, *, *, *}, *}, method: \"GET\", qs: \"\"}, {MyWSHandler, %{}, %MyWSHandler.State{...}})
...

Source: https://github.com/phoenixframework/websock_adapter/blob/c4c6ea66a1d65436be869abc6224b90317e4950a/lib/websock_adapter/cowboy_adapter.ex#L71

It looks like the terminate callback awaits 2-elem tuple in the 3rd argument: terminate(reason, _req, {handler, state}), but there are 3 elements - ws handler module, some empty map (%{}) and the WS handler state.

Package versions:

cowboy "2.11.0"
plug "1.16.1"
plug_cowboy "2.6.2"
websock "0.5.3"
websock_adapter "0.5.7"
elixir 1.15.6-otp-26
erlang 26.1.1
mtrudel commented 3 weeks ago

Thanks for this - could you provide a more complete stacktrace so I can better figure out where this is happening?

focused commented 3 weeks ago

[error] Ranch listener MyApp.Router.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.1203.0> exit with reason: {:function_clause, [{WebSockAdapter.CowboyAdapter, :terminate, [{:crash, :error, :function_clause}, %{port: 8080, scheme: "http", version: :"HTTP/1.1", path: "******", host: "127.0.0.1", peer: {{127, 0, 0, 1}, 57175}, method: "GET", qs: ""}, {MyApp.WSHandler, %{}, %MyApp.WSHandler.State{...}}], [file: ~c"lib/websock_adapter/cowboy_adapter.ex", line: 52]}, {:cowboy_websocket, :handler_call, 6, [file: ~c"/Users/d/Work/my-app/deps/cowboy/src/cowboy_websocket.erl", line: 564]}, {:cowboy_http, :loop, 1, [file: ~c"/Users/d/Work/my-app/deps/cowboy/src/cowboy_http.erl", line: 253]}, {:proc_lib, :init_p_do_apply, 3, [file: ~c"proc_lib.erl", line: 241]}]}

[error] Ranch listener MyApp.Router.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.1203.0> exit with reason: {:function_clause, [{WebSockAdapter.CowboyAdapter, :terminate, [{:crash, :error, :function_clause}, %{port: 8080, scheme: "http", version: :"HTTP/1.1", path: "******", host: "127.0.0.1", peer: {{127, 0, 0, 1}, 57175}, method: "GET", qs: ""}, {MyApp.WSHandler, %{}, %MyApp.WSHandler.State{...}}], [file: ~c"lib/websock_adapter/cowboy_adapter.ex", line: 52]}, {:cowboy_websocket, :handler_call, 6, [file: ~c"/Users/d/Work/my-app/deps/cowboy/src/cowboy_websocket.erl", line: 564]}, {:cowboy_http, :loop, 1, [file:  #~c"/Users/d/Work/my-app/deps/cowboy/src/cowboy_http.erl", line: 253]}, {:proc_lib, :init_p_do_apply, 3, [file: ~c"proc_lib.erl", line: 241]}]}
mtrudel commented 3 weeks ago

It looks like it's an issue with a crash during the upgrade process. It also looks like you may be returning tuple elements in the wrong order when you upgrade; I see {MyApp.WSHandler, %{}, %MyApp.WSHandler.State{...}} in the above stack trace, but the expected order is {handler_mod, handler_state, cowboy_opts}. Perhaps you have the last two elements reversed?

focused commented 3 weeks ago

It definitely happened after the upgrade in the WebSock.init/1 callback.

There was a clause error in the part not related to the ws, and the standard clause elixir error is passed to the terminate callback, which in turn raises the clause error also.

P.S.: after fixing the clause error bug in the internal module, the error gone, but it looks like the terminate callback would raise in case of some other error also.

mtrudel commented 3 weeks ago

Glad to hear you (sort of) got sorted.

It would be immensely helpful to have a minimal repro for this, either as a Garyfile or a repo

focused commented 3 weeks ago

Sure, I'll prepare it in a couple of days.

focused commented 2 weeks ago

https://github.com/focused/websock_adapter_app/tree/main

Just run mix test (maybe increase delay :timer.sleep(10)) or call it in console:

❯ iex -S mix
Erlang/OTP 27 [erts-15.1.1] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Interactive Elixir (1.17.3) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> {:ok, pid} = MyApp.WSClient.start_link()
{:ok, #PID<0.310.0>}
-------------------------------------------------------------: %MyApp.WSHandler.State{x: nil, y: nil}
** (EXIT from #PID<0.309.0>) shell process exited with reason: {:remote, 1011, ""}

22:50:48.257 [error] Ranch listener MyApp.Router.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.314.0> exit with reason: {:function_clause, [{WebSockAdapter.CowboyAdapter, :terminate, [{:crash, :error, %RuntimeError{message: "Custom Error"}}, %{port: 80, scheme: "http", version: :"HTTP/1.1", path: "/ws", host: "localhost", peer: {{127, 0, 0, 1}, 64162}, method: "GET", qs: ""}, {MyApp.WSHandler, %{}, %MyApp.WSHandler.State{x: nil, y: nil}}], [file: ~c"lib/websock_adapter/cowboy_adapter.ex", line: 52]}, {:cowboy_websocket, :handler_call, 6, [file: ~c"/Users/d/Code/my_app/deps/cowboy/src/cowboy_websocket.erl", line: 564]}, {:cowboy_http, :loop, 1, [file: ~c"/Users/d/Code/my_app/deps/cowboy/src/cowboy_http.erl", line: 253]}, {:proc_lib, :init_p_do_apply, 3, [file: ~c"proc_lib.erl", line: 329]}]}
mtrudel commented 1 week ago

Fix for this up at #20!