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

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

Open
focused opened this issue Oct 7, 2024 · 8 comments · May be fixed by #20
Open

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

focused opened this issue Oct 7, 2024 · 8 comments · May be fixed by #20

Comments

@focused
Copy link

focused commented Oct 7, 2024

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:

def terminate(reason, _req, {handler, state}) do

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
Copy link
Member

mtrudel commented Oct 7, 2024

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

@focused
Copy link
Author

focused commented Oct 7, 2024

[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
Copy link
Member

mtrudel commented Oct 10, 2024

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
Copy link
Author

focused commented Oct 10, 2024

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
Copy link
Member

mtrudel commented Oct 10, 2024

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
Copy link
Author

focused commented Oct 11, 2024

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

@focused
Copy link
Author

focused commented Oct 16, 2024

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 mtrudel linked a pull request Oct 25, 2024 that will close this issue
@mtrudel
Copy link
Member

mtrudel commented Oct 25, 2024

Fix for this up at #20!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants