From 25a62001dda70c0fe2d6f21556345a6e3885966f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 26 Feb 2019 13:17:11 -0500 Subject: [PATCH] Improve the failure case user experience And test it --- src/allmydata/test/test_runner.py | 24 ++++++++++++++++++++++++ src/allmydata/util/eliotutil.py | 20 ++++++++++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index c47670ca7..97df7404e 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -186,6 +186,30 @@ class BinTahoe(common_util.SignalMixin, unittest.TestCase, RunBinTahoeMixin): ]) self.assertEqual(rc_or_sig, 0) + @inlineCallbacks + def test_unknown_eliot_destination(self): + out, err, rc_or_sig = yield self.run_bintahoe([ + "--eliot-destination=invalid:more", + ]) + self.assertEqual(1, rc_or_sig) + self.assertIn("Unknown destination description", out) + self.assertIn("invalid:more", out) + + @inlineCallbacks + def test_malformed_eliot_destination(self): + out, err, rc_or_sig = yield self.run_bintahoe([ + "--eliot-destination=invalid", + ]) + self.assertEqual(1, rc_or_sig) + self.assertIn("must be formatted like", out) + + @inlineCallbacks + def test_escape_in_eliot_destination(self): + out, err, rc_or_sig = yield self.run_bintahoe([ + "--eliot-destination=file:\\foo", + ]) + self.assertEqual(1, rc_or_sig) + self.assertIn("Unsupported escape character", out) class CreateNode(unittest.TestCase): diff --git a/src/allmydata/util/eliotutil.py b/src/allmydata/util/eliotutil.py index 2e899741d..e49b02a5f 100644 --- a/src/allmydata/util/eliotutil.py +++ b/src/allmydata/util/eliotutil.py @@ -65,6 +65,9 @@ from eliot._validation import ( ValidationError, ) +from twisted.python.usage import ( + UsageError, +) from twisted.python.filepath import ( FilePath, ) @@ -287,9 +290,12 @@ def opt_eliot_destination(self, description): """ Add an Eliot logging destination. May be given more than once. """ - self.setdefault("destinations", []).append( - _parse_destination_description(description) - ) + try: + destination = _parse_destination_description(description) + except Exception as e: + raise UsageError(str(e)) + else: + self.setdefault("destinations", []).append(destination) def opt_help_eliot_destinations(self): @@ -399,7 +405,13 @@ class _DestinationParser(object): def parse(self, description): description = description.decode(u"ascii") - kind, args = description.split(u":", 1) + try: + kind, args = description.split(u":", 1) + except ValueError: + raise ValueError( + u"Eliot destination description must be formatted like " + u":." + ) try: parser = getattr(self, u"_parse_{}".format(kind)) except AttributeError: