From 13b4a1793fb21d4f35b4debc77c7ef4382d96ee8 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Sat, 1 Jul 2017 01:04:59 -0700 Subject: [PATCH] server: OPEN/CLOSE on crowded mailbox should provoke "crowded" error The Mailbox object throws CrowdedError, but WebSocketRendezvous wasn't handling it specifically. The server responded by dropping the connection and logging an "Unhandled Error", so the client would reconnect and then get the same error again and again. This changes WebSocketRendezvous to handle CrowdedError by sending a "crowded" error response. The client should react to this by giving up on the connection entirely, and not reconnecting. --- src/wormhole/server/rendezvous_websocket.py | 14 ++++++--- src/wormhole/test/test_server.py | 32 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/wormhole/server/rendezvous_websocket.py b/src/wormhole/server/rendezvous_websocket.py index dc4eb51..a5f8eb0 100644 --- a/src/wormhole/server/rendezvous_websocket.py +++ b/src/wormhole/server/rendezvous_websocket.py @@ -231,8 +231,11 @@ class WebSocketRendezvous(websocket.WebSocketServerProtocol): mailbox_id = msg["mailbox"] assert isinstance(mailbox_id, type("")) self._mailbox_id = mailbox_id - self._mailbox = self._app.open_mailbox(mailbox_id, self._side, - server_rx) + try: + self._mailbox = self._app.open_mailbox(mailbox_id, self._side, + server_rx) + except CrowdedError: + raise Error("crowded") def _send(sm): self.send("message", side=sm.side, phase=sm.phase, body=sm.body, server_rx=sm.server_rx, id=sm.msg_id) @@ -268,8 +271,11 @@ class WebSocketRendezvous(websocket.WebSocketServerProtocol): raise Error("close without mailbox must follow open") mailbox_id = self._mailbox_id if not self._mailbox: - self._mailbox = self._app.open_mailbox(mailbox_id, self._side, - server_rx) + try: + self._mailbox = self._app.open_mailbox(mailbox_id, self._side, + server_rx) + except CrowdedError: + raise Error("crowded") if self._listening: self._mailbox.remove_listener(self) self._listening = False diff --git a/src/wormhole/test/test_server.py b/src/wormhole/test/test_server.py index f27eb9f..514d573 100644 --- a/src/wormhole/test/test_server.py +++ b/src/wormhole/test/test_server.py @@ -892,6 +892,22 @@ class WebSocketAPI(_Util, ServerBase, unittest.TestCase): self.assertEqual(err["type"], "error") self.assertEqual(err["error"], "only one open per connection") + @inlineCallbacks + def test_open_crowded(self): + c1 = yield self.make_client() + yield c1.next_non_ack() + c1.send("bind", appid="appid", side="side") + app = self._rendezvous.get_app("appid") + + mbid = app.claim_nameplate("np1", "side1", 0) + app.claim_nameplate("np1", "side2", 0) + + # the third open will signal crowding + c1.send("open", mailbox=mbid) + err = yield c1.next_non_ack() + self.assertEqual(err["type"], "error") + self.assertEqual(err["error"], "crowded") + @inlineCallbacks def test_add(self): c1 = yield self.make_client() @@ -991,6 +1007,22 @@ class WebSocketAPI(_Util, ServerBase, unittest.TestCase): self.assertEqual(err["type"], "error") self.assertEqual(err["error"], "open and close must use same mailbox") + @inlineCallbacks + def test_close_crowded(self): + c1 = yield self.make_client() + yield c1.next_non_ack() + c1.send("bind", appid="appid", side="side") + app = self._rendezvous.get_app("appid") + + mbid = app.claim_nameplate("np1", "side1", 0) + app.claim_nameplate("np1", "side2", 0) + + # a close that allocates a third side will signal crowding + c1.send("close", mailbox=mbid) + err = yield c1.next_non_ack() + self.assertEqual(err["type"], "error") + self.assertEqual(err["error"], "crowded") + @inlineCallbacks def test_disconnect(self):