Add except clause to catch script parsing errors (#1929)

4 files changed, 23 insertions, 11 deletions

diff --git a/mitmproxy/addons/script.py b/mitmproxy/addons/script.py
index 93245760..07a8975a 100644
--- a/mitmproxy/addons/script.py
+++ b/mitmproxy/addons/script.py
@@ -20,7 +20,7 @@ def parse_command(command):
         Returns a (path, args) tuple.
     """
     if not command or not command.strip():
-        raise exceptions.OptionsError("Empty script command.")
+        raise ValueError("Empty script command.")
     # Windows: escape all backslashes in the path.
     if os.name == "nt":  # pragma: no cover
         backslashes = shlex.split(command, posix=False)[0].count("\\")
@@ -28,13 +28,13 @@ def parse_command(command):
     args = shlex.split(command)  # pragma: no cover
     args[0] = os.path.expanduser(args[0])
     if not os.path.exists(args[0]):
-        raise exceptions.OptionsError(
+        raise ValueError(
             ("Script file not found: %s.\r\n"
              "If your script path contains spaces, "
              "make sure to wrap it in additional quotes, e.g. -s \"'./foo bar/baz.py' --args\".") %
             args[0])
     elif os.path.isdir(args[0]):
-        raise exceptions.OptionsError("Not a file: %s" % args[0])
+        raise ValueError("Not a file: %s" % args[0])
     return args[0], args[1:]
 
 
@@ -205,7 +205,10 @@ class ScriptLoader:
         An addon that manages loading scripts from options.
     """
     def run_once(self, command, flows):
-        sc = Script(command)
+        try:
+            sc = Script(command)
+        except ValueError as e:
+            raise ValueError(str(e))
         sc.load_script()
         for f in flows:
             for evt, o in events.event_sequence(f):
@@ -246,7 +249,10 @@ class ScriptLoader:
                     ordered.append(current[s])
                 else:
                     ctx.log.info("Loading script: %s" % s)
-                    sc = Script(s)
+                    try:
+                        sc = Script(s)
+                    except ValueError as e:
+                        raise exceptions.OptionsError(str(e))
                     ordered.append(sc)
                     newscripts.append(sc)
 
diff --git a/mitmproxy/tools/console/master.py b/mitmproxy/tools/console/master.py
index 77b48e8d..66a9faba 100644
--- a/mitmproxy/tools/console/master.py
+++ b/mitmproxy/tools/console/master.py
@@ -146,8 +146,8 @@ class ConsoleMaster(master.Master):
         try:
             with self.handlecontext():
                 sc.run_once(command, [f])
-        except mitmproxy.exceptions.AddonError as e:
-            signals.add_log("Script error: %s" % e, "warn")
+        except ValueError as e:
+            signals.add_log("Input error: %s" % e, "warn")
 
     def toggle_eventlog(self):
         self.options.console_eventlog = not self.options.console_eventlog
diff --git a/test/mitmproxy/addons/test_script.py b/test/mitmproxy/addons/test_script.py
index f6fca23e..6e04d49d 100644
--- a/test/mitmproxy/addons/test_script.py
+++ b/test/mitmproxy/addons/test_script.py
@@ -58,10 +58,10 @@ def test_reloadhandler():
 
 class TestParseCommand:
     def test_empty_command(self):
-        with tutils.raises(exceptions.OptionsError):
+        with tutils.raises(ValueError):
             script.parse_command("")
 
-        with tutils.raises(exceptions.OptionsError):
+        with tutils.raises(ValueError):
             script.parse_command("  ")
 
     def test_no_script_file(self):
@@ -203,6 +203,7 @@ class TestScriptLoader:
         evts = [i[1] for i in sc.ns.call_log]
         assert evts == ['start', 'requestheaders', 'request', 'responseheaders', 'response', 'done']
 
+        f = tflow.tflow(resp=True)
         with m.handlecontext():
             tutils.raises(
                 "file not found",
diff --git a/test/mitmproxy/console/test_master.py b/test/mitmproxy/console/test_master.py
index fb3c2527..82cc6f9d 100644
--- a/test/mitmproxy/console/test_master.py
+++ b/test/mitmproxy/console/test_master.py
@@ -4,7 +4,6 @@ from mitmproxy.tools import console
 from mitmproxy import proxy
 from mitmproxy import options
 from mitmproxy.tools.console import common
-
 from .. import mastertest
 
 
@@ -27,7 +26,7 @@ def test_options():
 class TestMaster(mastertest.MasterTest):
     def mkmaster(self, **opts):
         if "verbosity" not in opts:
-            opts["verbosity"] = 0
+            opts["verbosity"] = 1
         o = options.Options(**opts)
         return console.master.ConsoleMaster(o, proxy.DummyServer())
 
@@ -37,6 +36,12 @@ class TestMaster(mastertest.MasterTest):
             self.dummy_cycle(m, 1, b"")
             assert len(m.view) == i
 
+    def test_run_script_once(self):
+        m = self.mkmaster()
+        f = tflow.tflow(resp=True)
+        m.run_script_once("nonexistent", [f])
+        assert "Input error" in str(m.logbuffer[0])
+
     def test_intercept(self):
         """regression test for https://github.com/mitmproxy/mitmproxy/issues/1605"""
         m = self.mkmaster(intercept="~b bar")