diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index 52bd8217..561c34b3 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -75,7 +75,15 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler): @classmethod def broadcast(cls, msg): for client in cls.clients: - client.write_message(msg) + # We could check for client.ws_connection, but we don't really + # care why the broadcast failed, we just want the rest of them + # to succeed, so catch everything. + try: + client.write_message(msg) + except Exception as e: + logger.debug('Broadcast of WebSocket message to %s failed: %s', + client.request.remote_ip, e) + # TODO: should this do the same cleanup as the on_message code? def initialize(self, core): self.jsonrpc = make_jsonrpc_wrapper(core) diff --git a/tests/http/test_handlers.py b/tests/http/test_handlers.py index 5c958d9a..5803adaf 100644 --- a/tests/http/test_handlers.py +++ b/tests/http/test_handlers.py @@ -2,8 +2,11 @@ from __future__ import absolute_import, unicode_literals import os +import mock + import tornado.testing import tornado.web +import tornado.websocket import mopidy from mopidy.http import handlers @@ -35,3 +38,47 @@ class StaticFileHandlerTest(tornado.testing.AsyncHTTPTestCase): response.headers['X-Mopidy-Version'], mopidy.__version__) self.assertEqual( response.headers['Cache-Control'], 'no-cache') + + +class WebSocketHandlerTest(tornado.testing.AsyncHTTPTestCase): + def get_app(self): + self.core = mock.Mock() + return tornado.web.Application([ + (r'/ws/?', handlers.WebSocketHandler, {'core': self.core}) + ]) + + def connection(self): + url = self.get_url('/ws').replace('http', 'ws') + return tornado.websocket.websocket_connect(url, self.io_loop) + + @tornado.testing.gen_test + def test_invalid_json_rpc_request_doesnt_crash_handler(self): + # An uncaught error would result in no message, so this is just a + # simplistic test to verify this. + conn = yield self.connection() + conn.write_message('invalid request') + message = yield conn.read_message() + self.assertTrue(message) + + @tornado.testing.gen_test + def test_broadcast_makes_it_to_client(self): + conn = yield self.connection() + handlers.WebSocketHandler.broadcast('message') + message = yield conn.read_message() + self.assertEqual(message, 'message') + + @tornado.testing.gen_test + def test_broadcast_to_client_that_just_closed_connection(self): + conn = yield self.connection() + conn.close() + handlers.WebSocketHandler.broadcast('message') + + @tornado.testing.gen_test + def test_broadcast_to_client_without_ws_connection_present(self): + yield self.connection() + # Tornado checks for ws_connection and raises WebSocketClosedError + # if it is missing, this test case simulates winning a race were + # this has happened but we have not yet been removed from clients. + for client in handlers.WebSocketHandler.clients: + client.ws_connection = None + handlers.WebSocketHandler.broadcast('message')