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

Already on GitHub? Sign in to your account

Handle WebSocket connection closing #223

Merged
merged 2 commits into from Feb 15, 2013

Conversation

Projects
None yet
2 participants
Contributor

dustalov commented Jan 13, 2013

I found an issue with WebSocket connection closing when client is connected and server tries to close the stream.

#!/usr/bin/env ruby
$:<< '../lib' << 'lib'

require 'goliath'
require 'goliath/websocket'

class Websocket < Goliath::WebSocket
  def on_open(env)
    env.logger.info("WS OPEN")
    env['subscription'] = env.channel.subscribe { |m| env.stream_send(m) }
  end

  def on_message(env, msg)
    env.logger.info("WS MESSAGE: #{msg}")
    env.channel << msg
    env.stream_close
  end

  def on_close(env)
    env.logger.info("WS CLOSED")
    env.channel.unsubscribe(env['subscription'])
  end

  def on_error(env, error)
    env.logger.error error
  end
end

Then start Goliath, connect, and send message from any WebSocket client. The following output would be produced:

eveel@tazik{~/Work/goliath/examples}% ./websocket-closing.rb -s 
[24165:INFO] 2013-01-14 05:46:06 :: Starting server on 0.0.0.0:9000 in development mode. Watch out for stones.
[24165:INFO] 2013-01-14 05:46:11 :: WS OPEN
[24165:INFO] 2013-01-14 05:46:12 :: WS MESSAGE: Rock it with HTML5 WebSocket
[24165:ERROR] 2013-01-14 05:46:12 :: undefined method `close_connection' for #<#<Class:0x000000020bbb00>:0x000000020bba10>
/usr/local/rvm/gems/ruby-1.9.3-p362@goliath/gems/em-websocket-0.3.8/lib/em-websocket/message_processor_06.rb:25:in `message'
/usr/local/rvm/gems/ruby-1.9.3-p362@goliath/gems/em-websocket-0.3.8/lib/em-websocket/framing07.rb:106:in `process_data'
/usr/local/rvm/gems/ruby-1.9.3-p362@goliath/gems/em-websocket-0.3.8/lib/em-websocket/handler.rb:28:in `receive_data'
/home/eveel/Work/goliath/lib/goliath/websocket.rb:17:in `on_body'
/home/eveel/Work/goliath/lib/goliath/request.rb:121:in `call'
/home/eveel/Work/goliath/lib/goliath/request.rb:121:in `parse'
/home/eveel/Work/goliath/lib/goliath/connection.rb:70:in `receive_data'
/usr/local/rvm/gems/ruby-1.9.3-p362@goliath/gems/eventmachine-1.0.0/lib/eventmachine.rb:187:in `run_machine'
/usr/local/rvm/gems/ruby-1.9.3-p362@goliath/gems/eventmachine-1.0.0/lib/eventmachine.rb:187:in `run'
/usr/local/rvm/gems/ruby-1.9.3-p362@goliath/gems/em-synchrony-1.0.2/lib/em-synchrony.rb:28:in `synchrony'
/home/eveel/Work/goliath/lib/goliath/server.rb:73:in `start'
/home/eveel/Work/goliath/lib/goliath/runner.rb:297:in `run_server'
/home/eveel/Work/goliath/lib/goliath/runner.rb:221:in `run'
/home/eveel/Work/goliath/lib/goliath/application.rb:120:in `run!'
/home/eveel/Work/goliath/lib/goliath/application.rb:142:in `block in <module:Goliath>'
[24165:INFO] 2013-01-14 05:46:12 :: Status: 500, Content-Length: 85, Response Time: 861.04ms
[24165:INFO] 2013-01-14 05:46:12 :: WS CLOSED

I'm unable to set up local development environment for Goliath, but this issue can be easily resolved by adding the close_connection method into the connection handler class.

Owner

igrigorik commented Jan 14, 2013

stream_close is not the right method to use in this context. To close the WS connection cleanly, the following method should be used: https://github.com/igrigorik/em-websocket/blob/master/lib/em-websocket/connection.rb#L53

The patch solves the immediate problem, but we should probably update the code to use the proper termination sequence.

Contributor

dustalov commented Jan 14, 2013

Maybe, this way?

Owner

igrigorik commented Jan 15, 2013

Have you verified that the TCP connection is actually closed? That should send the right termination frame, but it's not clear if the connection is also terminated.

igrigorik added a commit that referenced this pull request Feb 15, 2013

Merge pull request #223 from ustalov/websocket-closing
Handle WebSocket connection closing

@igrigorik igrigorik merged commit c1d250b into postrank-labs:master Feb 15, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment