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

:ssl_closed message not handled in close loop #56

Closed
Kraigie opened this issue May 4, 2018 · 1 comment
Closed

:ssl_closed message not handled in close loop #56

Kraigie opened this issue May 4, 2018 · 1 comment
Labels

Comments

@Kraigie
Copy link
Contributor

Kraigie commented May 4, 2018

Looks to be similar and tangentially related to #45.

Here's the relevant excerpt from my logs -

2018-05-03 22:39:32.637 [warn] websocket disconnected with reason {:remote, 1001, ""}, attempting reconnect
2018-05-03 22:39:32.919 [error] No handle_info/2 clause in Elixir.Nostrum.Shard.Session provided for {:ssl_closed, {:sslsocket, {:gen_tcp, #Port<0.16490>, :tls_connection, :undefined}, #PID<0.4982.0>}}

I'm able to trigger this by returning {:close, state} from a handle_cast/2 callback, although I get it when the remote party disconnects as well as is the case with the log above.

I've confirmed that the issue is happening in the close_loop/4 function by adding the following match to the receive. Besides the print it's all just copied from the :tcp_closed match 🙃.

{:ssl_closed, ^socket} ->
  IO.inspect "received ssl_closed in close loop"
  new_conn = %{conn | socket: nil}
  debug = Utils.sys_debug(debug, :closed, state)
  purge_timer(timer_ref, :"websockex_close_timeout")
  state = Map.delete(state, :timer_ref)
  on_disconnect(reason, parent, debug, %{state | conn: new_conn})

As expected this prints the message and seems to be handled correctly. If this is how it should be handled (I'm unsure if we need to do anything special for ssl connections) I can open a PR. I'd probably need a little guidance to properly set up a test though.

@Azolo
Copy link
Owner

Azolo commented May 6, 2018

Yeah, the tests are a messy glob, but nothing "special" per-say needs to happen except all the special things that :ssl does...

But yes, it seems to be that I missed handling closures of :ssl sockets. 😣

As for testing, something along the lines of this

test "can close a https connection", context do
  # Close the original socket
  WebSockex.cast(context.pid, :close)
  assert_receive :normal_remote_closed

  # Test HTTPS
  {:ok, {server_ref, url}} = WebSockex.TestServer.start_https(self())
  on_exit fn -> WebSockex.TestServer.shutdown(server_ref) end

  {:ok, _pid} = TestClient.start_link(url, %{})

  WebSockex.cast(pid, :close)
  assert_receive :normal_remote_closed  
end

@Azolo Azolo added the bug label Aug 31, 2018
@Azolo Azolo closed this as completed in b14fa10 Aug 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants