From 34116c7b1f31eed908f48ed3b94b91014aab3952 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Mon, 25 Apr 2016 17:11:52 -0700 Subject: [PATCH] CLI: document and return correct errors Also clean up test_scripts.PregeneratedCode: * fetch results from both sides at the same time * only check rc when using a subprocess, since the direct call doesn't use rc=0 anymore * no need to cancel the other side's Deferred when one errors * provide more information if stderr was non-empty --- src/wormhole/cli/cmd_receive.py | 15 +++++++++++---- src/wormhole/cli/cmd_send.py | 12 ++++++++++-- src/wormhole/test/test_scripts.py | 32 +++++++++++-------------------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/wormhole/cli/cmd_receive.py b/src/wormhole/cli/cmd_receive.py index d1b2ca2..4e80567 100644 --- a/src/wormhole/cli/cmd_receive.py +++ b/src/wormhole/cli/cmd_receive.py @@ -14,6 +14,13 @@ class RespondError(Exception): self.response = response def receive(args, reactor=reactor): + """I implement 'wormhole receive'. I return a Deferred that fires with + None (for success), or signals one of the following errors: + * WrongPasswordError: the two sides didn't use matching passwords + * Timeout: something didn't happen fast enough for our tastes + * TransferError: the sender rejected the transfer: verifier mismatch + * any other error: something unexpected happened + """ return TwistedReceiver(args, reactor).go() @@ -72,7 +79,7 @@ class TwistedReceiver: try: if "message" in them_d: yield self.handle_text(them_d, w) - returnValue(0) + returnValue(None) if "file" in them_d: f = self.handle_file(them_d) rp = yield self.establish_transit(w, them_d, tor_manager) @@ -92,8 +99,8 @@ class TwistedReceiver: except RespondError as r: data = json.dumps(r.response).encode("utf-8") yield w.send_data(data) - raise SystemExit(1) - returnValue(0) + raise TransferError(r["error"]) + returnValue(None) @inlineCallbacks def handle_code(self, w): @@ -227,7 +234,7 @@ class TwistedReceiver: self.msg() self.msg(u"Connection dropped before full file received") self.msg(u"got %d bytes, wanted %d" % (received, self.xfersize)) - returnValue(1) # TODO: exit properly + raise TransferError("Connection dropped before full file received") assert received == self.xfersize def write_file(self, f): diff --git a/src/wormhole/cli/cmd_send.py b/src/wormhole/cli/cmd_send.py index 29740cd..d5af074 100644 --- a/src/wormhole/cli/cmd_send.py +++ b/src/wormhole/cli/cmd_send.py @@ -12,6 +12,14 @@ APPID = u"lothar.com/wormhole/text-or-file-xfer" @inlineCallbacks def send(args, reactor=reactor): + """I implement 'wormhole send'. I return a Deferred that fires with None + (for success), or signals one of the following errors: + * WrongPasswordError: the two sides didn't use matching passwords + * Timeout: something didn't happen fast enough for our tastes + * TransferError: the receiver rejected the transfer: verifier mismatch, + permission not granted, ack not successful. + * any other error: something unexpected happened + """ assert isinstance(args.relay_url, type(u"")) if args.zeromode: assert not args.code @@ -96,7 +104,7 @@ def send(args, reactor=reactor): if them_phase1["message_ack"] == "ok": print(u"text message sent", file=args.stdout) yield w.close() - returnValue(0) # terminates this function + returnValue(None) # terminates this function raise TransferError("error sending text: %r" % (them_phase1,)) if "error" in them_phase1: @@ -109,7 +117,7 @@ def send(args, reactor=reactor): yield w.close() yield _send_file_twisted(tdata, transit_sender, fd_to_send, args.stdout, args.hide_progress, args.timing) - returnValue(0) + returnValue(None) def build_phase1_data(args): phase1 = {} diff --git a/src/wormhole/test/test_scripts.py b/src/wormhole/test/test_scripts.py index 7f65de6..d24a989 100644 --- a/src/wormhole/test/test_scripts.py +++ b/src/wormhole/test/test_scripts.py @@ -3,7 +3,7 @@ import os, sys, re, io, zipfile, six from twisted.trial import unittest from twisted.python import procutils, log from twisted.internet.utils import getProcessOutputAndValue -from twisted.internet.defer import inlineCallbacks +from twisted.internet.defer import gatherResults, inlineCallbacks from .. import __version__ from .common import ServerBase from ..cli import runner, cmd_send, cmd_receive @@ -306,15 +306,17 @@ class PregeneratedCode(ServerBase, ScriptsBase, unittest.TestCase): path=send_dir) receive_d = getProcessOutputAndValue(wormhole_bin, receive_args, path=receive_dir) - send_res = yield send_d + (send_res, receive_res) = yield gatherResults([send_d, receive_d], + True) send_stdout = send_res[0].decode("utf-8") send_stderr = send_res[1].decode("utf-8") send_rc = send_res[2] - receive_res = yield receive_d receive_stdout = receive_res[0].decode("utf-8") receive_stderr = receive_res[1].decode("utf-8") receive_rc = receive_res[2] NL = os.linesep + self.assertEqual((send_rc, receive_rc), (0, 0), + (send_res, receive_res)) else: sargs = runner.parser.parse_args(send_args) sargs.cwd = send_dir @@ -330,22 +332,11 @@ class PregeneratedCode(ServerBase, ScriptsBase, unittest.TestCase): receive_d = cmd_receive.receive(rargs) # The sender might fail, leaving the receiver hanging, or vice - # versa. If either side fails, cancel the other, so it won't - # matter which Deferred we wait upon first. + # versa. Make sure we don't wait on one side exclusively - def _oops(f, which): - log.msg("test_scripts: %s failed, cancelling both" % which) - send_d.cancel() - receive_d.cancel() - return f - send_d.addErrback(_oops, "send_d") - receive_d.addErrback(_oops, "receive_d") - - send_rc = yield send_d + yield gatherResults([send_d, receive_d], True) send_stdout = sargs.stdout.getvalue() send_stderr = sargs.stderr.getvalue() - - receive_rc = yield receive_d receive_stdout = rargs.stdout.getvalue() receive_stderr = rargs.stderr.getvalue() @@ -355,8 +346,10 @@ class PregeneratedCode(ServerBase, ScriptsBase, unittest.TestCase): self.maxDiff = None # show full output for assertion failures - self.failUnlessEqual(send_stderr, "") - self.failUnlessEqual(receive_stderr, "") + self.failUnlessEqual(send_stderr, "", + (send_stdout, send_stderr)) + self.failUnlessEqual(receive_stderr, "", + (receive_stdout, receive_stderr)) # check sender if mode == "text": @@ -417,9 +410,6 @@ class PregeneratedCode(ServerBase, ScriptsBase, unittest.TestCase): with open(fn, "r") as f: self.failUnlessEqual(f.read(), message(i)) - self.failUnlessEqual(send_rc, 0) - self.failUnlessEqual(receive_rc, 0) - def test_text(self): return self._do_test() def test_text_subprocess(self):