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.
This commit is contained in:
Brian Warner 2017-07-01 01:04:59 -07:00
parent c58172632c
commit 13b4a1793f
2 changed files with 42 additions and 4 deletions

View File

@ -231,8 +231,11 @@ class WebSocketRendezvous(websocket.WebSocketServerProtocol):
mailbox_id = msg["mailbox"] mailbox_id = msg["mailbox"]
assert isinstance(mailbox_id, type("")) assert isinstance(mailbox_id, type(""))
self._mailbox_id = mailbox_id self._mailbox_id = mailbox_id
try:
self._mailbox = self._app.open_mailbox(mailbox_id, self._side, self._mailbox = self._app.open_mailbox(mailbox_id, self._side,
server_rx) server_rx)
except CrowdedError:
raise Error("crowded")
def _send(sm): def _send(sm):
self.send("message", side=sm.side, phase=sm.phase, self.send("message", side=sm.side, phase=sm.phase,
body=sm.body, server_rx=sm.server_rx, id=sm.msg_id) 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") raise Error("close without mailbox must follow open")
mailbox_id = self._mailbox_id mailbox_id = self._mailbox_id
if not self._mailbox: if not self._mailbox:
try:
self._mailbox = self._app.open_mailbox(mailbox_id, self._side, self._mailbox = self._app.open_mailbox(mailbox_id, self._side,
server_rx) server_rx)
except CrowdedError:
raise Error("crowded")
if self._listening: if self._listening:
self._mailbox.remove_listener(self) self._mailbox.remove_listener(self)
self._listening = False self._listening = False

View File

@ -892,6 +892,22 @@ class WebSocketAPI(_Util, ServerBase, unittest.TestCase):
self.assertEqual(err["type"], "error") self.assertEqual(err["type"], "error")
self.assertEqual(err["error"], "only one open per connection") 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 @inlineCallbacks
def test_add(self): def test_add(self):
c1 = yield self.make_client() c1 = yield self.make_client()
@ -991,6 +1007,22 @@ class WebSocketAPI(_Util, ServerBase, unittest.TestCase):
self.assertEqual(err["type"], "error") self.assertEqual(err["type"], "error")
self.assertEqual(err["error"], "open and close must use same mailbox") 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 @inlineCallbacks
def test_disconnect(self): def test_disconnect(self):