mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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.
This commit is contained in:
parent
dda31d59a0
commit
4f8a8e2c06
5 changed files with 136 additions and 1 deletions
|
@ -71,6 +71,8 @@ module ActionCable
|
||||||
|
|
||||||
def write(data)
|
def write(data)
|
||||||
@stream.write(data)
|
@stream.write(data)
|
||||||
|
rescue => e
|
||||||
|
emit_error e.message
|
||||||
end
|
end
|
||||||
|
|
||||||
def transmit(message)
|
def transmit(message)
|
||||||
|
|
|
@ -36,6 +36,7 @@ module ActionCable
|
||||||
@faye.on(:open) { |event| @event_target.on_open }
|
@faye.on(:open) { |event| @event_target.on_open }
|
||||||
@faye.on(:message) { |event| @event_target.on_message(event.data) }
|
@faye.on(:message) { |event| @event_target.on_message(event.data) }
|
||||||
@faye.on(:close) { |event| @event_target.on_close(event.reason, event.code) }
|
@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
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -29,7 +29,7 @@ module ActionCable
|
||||||
def write(data)
|
def write(data)
|
||||||
return @rack_hijack_io.write(data) if @rack_hijack_io
|
return @rack_hijack_io.write(data) if @rack_hijack_io
|
||||||
return @stream_send.call(data) if @stream_send
|
return @stream_send.call(data) if @stream_send
|
||||||
rescue EOFError
|
rescue EOFError, Errno::ECONNRESET
|
||||||
@socket_object.client_gone
|
@socket_object.client_gone
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
65
actioncable/test/connection/client_socket_test.rb
Normal file
65
actioncable/test/connection/client_socket_test.rb
Normal file
|
@ -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
|
67
actioncable/test/connection/stream_test.rb
Normal file
67
actioncable/test/connection/stream_test.rb
Normal file
|
@ -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
|
Loading…
Reference in a new issue