aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJeremy Daer <jeremydaer@gmail.com>2016-03-20 13:59:02 -0700
committerJeremy Daer <jeremydaer@gmail.com>2016-03-20 17:36:22 -0700
commit4f8a8e2c065e4db1f4df0df2fd75338154c6be07 (patch)
treebfa7e833c7c8a4cc3d7c7af714fe06a274e3ebbe
parentdda31d59a03adb7e5aa372e72bb66a3886632f2f (diff)
downloadrails-4f8a8e2c065e4db1f4df0df2fd75338154c6be07.tar.gz
rails-4f8a8e2c065e4db1f4df0df2fd75338154c6be07.tar.bz2
rails-4f8a8e2c065e4db1f4df0df2fd75338154c6be07.zip
Gracefully handle disconnected clients
We'll get `Errno::ECONNRESET` if the client forcibly disconnected. Just close the socket rather than raising the exception. Handle other errors in `ClientSocket#write`, too, mirroring the Faye error handling which swallows all `StandardError` on write.
-rw-r--r--actioncable/lib/action_cable/connection/client_socket.rb2
-rw-r--r--actioncable/lib/action_cable/connection/faye_client_socket.rb1
-rw-r--r--actioncable/lib/action_cable/connection/stream.rb2
-rw-r--r--actioncable/test/connection/client_socket_test.rb65
-rw-r--r--actioncable/test/connection/stream_test.rb67
5 files changed, 136 insertions, 1 deletions
diff --git a/actioncable/lib/action_cable/connection/client_socket.rb b/actioncable/lib/action_cable/connection/client_socket.rb
index 9e4dbcd6e6..7d6de78582 100644
--- a/actioncable/lib/action_cable/connection/client_socket.rb
+++ b/actioncable/lib/action_cable/connection/client_socket.rb
@@ -71,6 +71,8 @@ module ActionCable
def write(data)
@stream.write(data)
+ rescue => e
+ emit_error e.message
end
def transmit(message)
diff --git a/actioncable/lib/action_cable/connection/faye_client_socket.rb b/actioncable/lib/action_cable/connection/faye_client_socket.rb
index c9139b6858..47d09a9e14 100644
--- a/actioncable/lib/action_cable/connection/faye_client_socket.rb
+++ b/actioncable/lib/action_cable/connection/faye_client_socket.rb
@@ -36,6 +36,7 @@ module ActionCable
@faye.on(:open) { |event| @event_target.on_open }
@faye.on(:message) { |event| @event_target.on_message(event.data) }
@faye.on(:close) { |event| @event_target.on_close(event.reason, event.code) }
+ @faye.on(:error) { |event| @event_target.on_error(event.message) }
end
end
end
diff --git a/actioncable/lib/action_cable/connection/stream.rb b/actioncable/lib/action_cable/connection/stream.rb
index 2d97b28c09..0cf59091bc 100644
--- a/actioncable/lib/action_cable/connection/stream.rb
+++ b/actioncable/lib/action_cable/connection/stream.rb
@@ -29,7 +29,7 @@ module ActionCable
def write(data)
return @rack_hijack_io.write(data) if @rack_hijack_io
return @stream_send.call(data) if @stream_send
- rescue EOFError
+ rescue EOFError, Errno::ECONNRESET
@socket_object.client_gone
end
diff --git a/actioncable/test/connection/client_socket_test.rb b/actioncable/test/connection/client_socket_test.rb
new file mode 100644
index 0000000000..dd730e348f
--- /dev/null
+++ b/actioncable/test/connection/client_socket_test.rb
@@ -0,0 +1,65 @@
+require 'test_helper'
+require 'stubs/test_server'
+
+class ActionCable::Connection::StreamTest < ActionCable::TestCase
+ class Connection < ActionCable::Connection::Base
+ attr_reader :websocket, :subscriptions, :message_buffer, :connected
+ attr_reader :errors
+
+ def initialize(*)
+ super
+ @errors = []
+ end
+
+ def connect
+ @connected = true
+ end
+
+ def disconnect
+ @connected = false
+ end
+
+ def send_async(method, *args)
+ send method, *args
+ end
+
+ def on_error(message)
+ @errors << message
+ end
+ end
+
+ setup do
+ @server = TestServer.new
+ @server.config.allowed_request_origins = %w( http://rubyonrails.com )
+ end
+
+ test 'delegate socket errors to on_error handler' do
+ skip if ENV['FAYE'].present?
+
+ run_in_eventmachine do
+ connection = open_connection
+
+ # Internal hax = :(
+ client = connection.websocket.send(:websocket)
+ client.instance_variable_get('@stream').expects(:write).raises('foo')
+ client.expects(:client_gone).never
+
+ client.write('boo')
+ assert_equal %w[ foo ], connection.errors
+ end
+ end
+
+ private
+ def open_connection
+ env = Rack::MockRequest.env_for '/test',
+ 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket',
+ 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.com'
+ env['rack.hijack'] = -> { env['rack.hijack_io'] = StringIO.new }
+
+ Connection.new(@server, env).tap do |connection|
+ connection.process
+ connection.send :handle_open
+ assert connection.connected
+ end
+ end
+end
diff --git a/actioncable/test/connection/stream_test.rb b/actioncable/test/connection/stream_test.rb
new file mode 100644
index 0000000000..d5aad63648
--- /dev/null
+++ b/actioncable/test/connection/stream_test.rb
@@ -0,0 +1,67 @@
+require 'test_helper'
+require 'stubs/test_server'
+
+class ActionCable::Connection::StreamTest < ActionCable::TestCase
+ class Connection < ActionCable::Connection::Base
+ attr_reader :websocket, :subscriptions, :message_buffer, :connected
+ attr_reader :errors
+
+ def initialize(*)
+ super
+ @errors = []
+ end
+
+ def connect
+ @connected = true
+ end
+
+ def disconnect
+ @connected = false
+ end
+
+ def send_async(method, *args)
+ send method, *args
+ end
+
+ def on_error(message)
+ @errors << message
+ end
+ end
+
+ setup do
+ @server = TestServer.new
+ @server.config.allowed_request_origins = %w( http://rubyonrails.com )
+ end
+
+ [ EOFError, Errno::ECONNRESET ].each do |closed_exception|
+ test "closes socket on #{closed_exception}" do
+ skip if ENV['FAYE'].present?
+
+ run_in_eventmachine do
+ connection = open_connection
+
+ # Internal hax = :(
+ client = connection.websocket.send(:websocket)
+ client.instance_variable_get('@stream').instance_variable_get('@rack_hijack_io').expects(:write).raises(closed_exception, 'foo')
+ client.expects(:client_gone)
+
+ client.write('boo')
+ assert_equal [], connection.errors
+ end
+ end
+ end
+
+ private
+ def open_connection
+ env = Rack::MockRequest.env_for '/test',
+ 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket',
+ 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.com'
+ env['rack.hijack'] = -> { env['rack.hijack_io'] = StringIO.new }
+
+ Connection.new(@server, env).tap do |connection|
+ connection.process
+ connection.send :handle_open
+ assert connection.connected
+ end
+ end
+end