improve error handling

errors raised while processing a received message will cause the Wormhole to
close-with-error, and any pending Deferreds will be errbacked

docs/boss.dot

@@ -50,13 +50,17 @@ digraph {
         S2 -> P_close_scary [label="scared" color="red"]
 
         S_closing [label="closing"]
-        S_closing -> P_closed [label="closed"]
+        S_closing -> P_closed [label="closed\nerror"]
         S_closing -> S_closing [label="got_message\nhappy\nscared\nclose"]
 
         P_closed [shape="box" label="W.closed(reason)"]
         P_closed -> S_closed
         S_closed [label="closed"]
 
+        S0 -> P_closed [label="error"]
+        S1 -> P_closed [label="error"]
+        S2 -> P_closed [label="error"]
+
         {rank=same; Other S_closed}
         Other [shape="box" style="dashed"
                label="rx_welcome -> process\nsend -> S.send\ngot_verifier -> W.got_verifier\nallocate -> C.allocate\ninput -> C.input\nset_code -> C.set_code"

docs/machines.dot

@@ -25,7 +25,7 @@ digraph {
 
         #Boss -> Connection [color="blue"]
         Boss -> Connection [style="dashed" label="start"]
-        Connection -> Boss [style="dashed" label="rx_welcome\nrx_error"]
+        Connection -> Boss [style="dashed" label="rx_welcome\nrx_error\nerror"]
 
         Boss -> Send [style="dashed" label="send"]
 

src/wormhole/_boss.py

@@ -17,12 +17,9 @@ from ._rendezvous import RendezvousConnector
 from ._nameplate_lister import NameplateListing
 from ._code import Code
 from ._terminator import Terminator
-from .errors import WrongPasswordError
+from .errors import ServerError, LonelyError, WrongPasswordError
 from .util import bytes_to_dict
 
-class WormholeError(Exception):
-    pass
-
 @attrs
 @implementer(_interfaces.IBoss)
 class Boss(object):
@@ -121,9 +118,15 @@ class Boss(object):
     @m.input()
     def close(self): pass
 
-    # from RendezvousConnector
+    # from RendezvousConnector. rx_error an error message from the server
+    # (probably because of something we did, or due to CrowdedError). error
+    # is when an exception happened while it tried to deliver something else
     @m.input()
     def rx_welcome(self, welcome): pass
+    @m.input()
+    def rx_error(self, errmsg, orig): pass
+    @m.input()
+    def error(self, err): pass
 
     # from Code (provoked by input/allocate/set_code)
     @m.input()
@@ -135,8 +138,6 @@ class Boss(object):
     def happy(self): pass
     @m.input()
     def scared(self): pass
-    @m.input()
-    def rx_error(self, err, orig): pass
 
     def got_message(self, phase, plaintext):
         assert isinstance(phase, type("")), type(phase)
@@ -184,8 +185,8 @@ class Boss(object):
         self._S.send("%d" % phase, plaintext)
 
     @m.output()
-    def close_error(self, err, orig):
+    def close_error(self, errmsg, orig):
-        self._result = WormholeError(err)
+        self._result = ServerError(errmsg)
         self._T.close("errory")
     @m.output()
     def close_scared(self):
@@ -193,7 +194,7 @@ class Boss(object):
         self._T.close("scary")
     @m.output()
     def close_lonely(self):
-        self._result = WormholeError("lonely")
+        self._result = LonelyError()
         self._T.close("lonely")
     @m.output()
     def close_happy(self):
@@ -212,8 +213,14 @@ class Boss(object):
             self._W.received(self._rx_phases.pop(self._next_rx_phase))
             self._next_rx_phase += 1
 
+    @m.output()
+    def W_close_with_error(self, err):
+        self._result = err # exception
+        self._W.closed(self._result)
+
     @m.output()
     def W_closed(self):
+        # result is either "happy" or a WormholeError of some sort
         self._W.closed(self._result)
 
     S0_empty.upon(close, enter=S3_closing, outputs=[close_lonely])
@@ -221,6 +228,8 @@ class Boss(object):
     S0_empty.upon(rx_welcome, enter=S0_empty, outputs=[process_welcome])
     S0_empty.upon(got_code, enter=S1_lonely, outputs=[do_got_code])
     S0_empty.upon(rx_error, enter=S3_closing, outputs=[close_error])
+    S0_empty.upon(error, enter=S4_closed, outputs=[W_close_with_error])
+
     S1_lonely.upon(rx_welcome, enter=S1_lonely, outputs=[process_welcome])
     S1_lonely.upon(happy, enter=S2_happy, outputs=[])
     S1_lonely.upon(scared, enter=S3_closing, outputs=[close_scared])
@@ -228,6 +237,8 @@ class Boss(object):
     S1_lonely.upon(send, enter=S1_lonely, outputs=[S_send])
     S1_lonely.upon(got_verifier, enter=S1_lonely, outputs=[W_got_verifier])
     S1_lonely.upon(rx_error, enter=S3_closing, outputs=[close_error])
+    S1_lonely.upon(error, enter=S4_closed, outputs=[W_close_with_error])
+
     S2_happy.upon(rx_welcome, enter=S2_happy, outputs=[process_welcome])
     S2_happy.upon(got_phase, enter=S2_happy, outputs=[W_received])
     S2_happy.upon(got_version, enter=S2_happy, outputs=[process_version])
@@ -235,6 +246,7 @@ class Boss(object):
     S2_happy.upon(close, enter=S3_closing, outputs=[close_happy])
     S2_happy.upon(send, enter=S2_happy, outputs=[S_send])
     S2_happy.upon(rx_error, enter=S3_closing, outputs=[close_error])
+    S2_happy.upon(error, enter=S4_closed, outputs=[W_close_with_error])
 
     S3_closing.upon(rx_welcome, enter=S3_closing, outputs=[])
     S3_closing.upon(rx_error, enter=S3_closing, outputs=[])
@@ -245,6 +257,7 @@ class Boss(object):
     S3_closing.upon(close, enter=S3_closing, outputs=[])
     S3_closing.upon(send, enter=S3_closing, outputs=[])
     S3_closing.upon(closed, enter=S4_closed, outputs=[W_closed])
+    S3_closing.upon(error, enter=S4_closed, outputs=[W_close_with_error])
 
     S4_closed.upon(rx_welcome, enter=S4_closed, outputs=[])
     S4_closed.upon(got_phase, enter=S4_closed, outputs=[])
@@ -253,4 +266,4 @@ class Boss(object):
     S4_closed.upon(scared, enter=S4_closed, outputs=[])
     S4_closed.upon(close, enter=S4_closed, outputs=[])
     S4_closed.upon(send, enter=S4_closed, outputs=[])
-
+    S4_closed.upon(error, enter=S4_closed, outputs=[])

src/wormhole/_rendezvous.py

@@ -164,7 +164,11 @@ class RendezvousConnector(object):
             # make tests fail, but real application will ignore it
             log.err(ValueError("Unknown inbound message type %r" % (msg,)))
             return
-        return meth(msg)
+        try:
+            return meth(msg)
+        except Exception as e:
+            self._B.error(e)
+            raise
 
     def ws_close(self, wasClean, code, reason):
         self._debug("R.lost")
@@ -211,6 +215,10 @@ class RendezvousConnector(object):
         pass
 
     def _response_handle_error(self, msg):
+        # the server sent us a type=error. Most cases are due to our mistakes
+        # (malformed protocol messages, sending things in the wrong order),
+        # but it can also result from CrowdedError (more than two clients
+        # using the same channel).
         err = msg["error"]
         orig = msg["orig"]
         self._B.rx_error(err, orig)

src/wormhole/errors.py

@@ -1,33 +1,25 @@
 from __future__ import unicode_literals
-import functools
 
-class ServerError(Exception):
+class WormholeError(Exception):
-    def __init__(self, message, relay):
+    """Parent class for all wormhole-related errors"""
-        self.message = message
-        self.relay = relay
-    def __str__(self):
-        return self.message
 
-def handle_server_error(func):
+class ServerError(WormholeError):
-    @functools.wraps(func)
+    """The relay server complained about something we did."""
-    def _wrap(*args, **kwargs):
-        try:
-            return func(*args, **kwargs)
-        except ServerError as e:
-            print("Server error (from %s):\n%s" % (e.relay, e.message))
-            return 1
-    return _wrap
 
-class Timeout(Exception):
+class Timeout(WormholeError):
     pass
 
-class WelcomeError(Exception):
+class WelcomeError(WormholeError):
     """
     The relay server told us to signal an error, probably because our version
     is too old to possibly work. The server said:"""
     pass
 
-class WrongPasswordError(Exception):
+class LonelyError(WormholeError):
+    """wormhole.close() was called before the peer connection could be
+    established"""
+
+class WrongPasswordError(WormholeError):
     """
     Key confirmation failed. Either you or your correspondent typed the code
     wrong, or a would-be man-in-the-middle attacker guessed incorrectly. You
@@ -37,24 +29,24 @@ class WrongPasswordError(Exception):
     # or the data blob was corrupted, and that's why decrypt failed
     pass
 
-class KeyFormatError(Exception):
+class KeyFormatError(WormholeError):
     """
     The key you entered contains spaces. Magic-wormhole expects keys to be
     separated by dashes. Please reenter the key you were given separating the
     words with dashes.
     """
 
-class ReflectionAttack(Exception):
+class ReflectionAttack(WormholeError):
     """An attacker (or bug) reflected our outgoing message back to us."""
 
-class InternalError(Exception):
+class InternalError(WormholeError):
     """The programmer did something wrong."""
 
 class WormholeClosedError(InternalError):
     """API calls may not be made after close() is called."""
 
-class TransferError(Exception):
+class TransferError(WormholeError):
     """Something bad happened and the transfer failed."""
 
-class NoTorError(Exception):
+class NoTorError(WormholeError):
     """--tor was requested, but 'txtorcon' is not installed."""

src/wormhole/test/test_wormhole_new.py

@@ -1,9 +1,10 @@
 from __future__ import print_function, unicode_literals
+import re
 from twisted.trial import unittest
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks
 from .common import ServerBase
-from .. import wormhole
+from .. import wormhole, errors
 
 APPID = "appid"
 
@@ -23,15 +24,19 @@ class Delegate:
         self.closed = result
 
 class New(ServerBase, unittest.TestCase):
+    timeout = 2
+
     @inlineCallbacks
     def test_allocate(self):
         w = wormhole.deferred_wormhole(APPID, self.relayurl, reactor)
-        w.debug_set_trace("W1")
+        #w.debug_set_trace("W1")
         w.allocate_code(2)
         code = yield w.when_code()
-        print("code:", code)
+        self.assertEqual(type(code), type(""))
-        yield w.close()
+        mo = re.search(r"^\d+-\w+-\w+$", code)
-    test_allocate.timeout = 2
+        self.assert_(mo, code)
+        # w.close() fails because we closed before connecting
+        self.assertFailure(w.close(), errors.LonelyError)
 
     def test_delegated(self):
         dg = Delegate()
@@ -41,10 +46,11 @@ class New(ServerBase, unittest.TestCase):
     @inlineCallbacks
     def test_basic(self):
         w1 = wormhole.deferred_wormhole(APPID, self.relayurl, reactor)
-        w1.debug_set_trace("W1")
+        #w1.debug_set_trace("W1")
         w1.allocate_code(2)
         code = yield w1.when_code()
-        print("code:", code)
+        mo = re.search(r"^\d+-\w+-\w+$", code)
+        self.assert_(mo, code)
         w2 = wormhole.deferred_wormhole(APPID, self.relayurl, reactor)
         w2.set_code(code)
         code2 = yield w2.when_code()
@@ -55,6 +61,28 @@ class New(ServerBase, unittest.TestCase):
         data = yield w2.when_received()
         self.assertEqual(data, b"data")
 
-        yield w1.close()
+        w2.send(b"data2")
-        yield w2.close()
+        data2 = yield w1.when_received()
-    test_basic.timeout = 2
+        self.assertEqual(data2, b"data2")
+
+        c1 = yield w1.close()
+        self.assertEqual(c1, "happy")
+        c2 = yield w2.close()
+        self.assertEqual(c2, "happy")
+
+    @inlineCallbacks
+    def test_wrong_password(self):
+        w1 = wormhole.deferred_wormhole(APPID, self.relayurl, reactor)
+        #w1.debug_set_trace("W1")
+        w1.allocate_code(2)
+        code = yield w1.when_code()
+        w2 = wormhole.deferred_wormhole(APPID, self.relayurl, reactor)
+        w2.set_code(code+", NOT")
+        code2 = yield w2.when_code()
+        self.assertNotEqual(code, code2)
+
+        w1.send(b"data")
+
+        self.assertFailure(w2.when_received(), errors.WrongPasswordError)
+        self.assertFailure(w1.close(), errors.WrongPasswordError)
+        self.assertFailure(w2.close(), errors.WrongPasswordError)

src/wormhole/wormhole.py

@@ -2,12 +2,13 @@ from __future__ import print_function, absolute_import, unicode_literals
 import os, sys
 from attr import attrs, attrib
 from zope.interface import implementer
+from twisted.python import failure
 from twisted.internet import defer
 from ._interfaces import IWormhole
 from .util import bytes_to_hexstr
 from .timing import DebugTiming
 from .journal import ImmediateJournal
-from ._boss import Boss, WormholeError
+from ._boss import Boss
 
 # We can provide different APIs to different apps:
 # * Deferreds
@@ -118,6 +119,9 @@ class _DeferredWormhole(object):
     def send(self, plaintext):
         self._boss.send(plaintext)
     def close(self):
+        # fails with WormholeError unless we established a connection
+        # (state=="happy"). Fails with WrongPasswordError (a subclass of
+        # WormholeError) if state=="scary".
         self._boss.close()
         d = defer.Deferred()
         self._closed_observers.append(d)
@@ -146,17 +150,20 @@ class _DeferredWormhole(object):
         self._received_data.append(plaintext)
 
     def closed(self, result):
-        print("closed", result, type(result))
+        #print("closed", result, type(result))
-        if isinstance(result, WormholeError):
+        if isinstance(result, Exception):
-            e = result
+            observer_result = close_result = failure.Failure(result)
         else:
-            e = WormholeClosed(result)
+            # pending w.verify() or w.read() get an error
+            observer_result = WormholeClosed(result)
+            # but w.close() only gets error if we're unhappy
+            close_result = result
         for d in self._verifier_observers:
-            d.errback(e)
+            d.errback(observer_result)
         for d in self._received_observers:
-            d.errback(e)
+            d.errback(observer_result)
         for d in self._closed_observers:
-            d.callback(result)
+            d.callback(close_result)
 
 def _wormhole(appid, relay_url, reactor, delegate=None,
               tor_manager=None, timing=None,