Repository: qutebrowser/qutebrowser
Pull request: improved error messages for inexistent userscripts
fixes #1940

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/qutebrowser/qutebrowser/1994)
<!-- Reviewable:end -->


Problem statement:
Check that userscript exists before attempting to execute it
If a userscript is invoked by the `:spawn --userscript myscript` command, but `myscript` does not exist in the userscripts directory, qutebrowser gives the uninformative message:

```
Error while spawning userscript: The process failed to start.
```

The log output is similarly opaque. The log output does make it clear that qutebrowser simply assumes the existence of the userscript and attempts to execute it.

It would be more useful if qutebrowser checked that the userscript exists in the userscript directory before attempting to execute it. If the userscript cannot be found qutebrowser could provide a specific, more helpful, error message like:

```
Userscript 'myscript' not found in userscript directory 'path/to/userscript/directory'
```

Hints from issue discussion:

Commit message: improved error messages for inexistent userscripts

fixes #1940

Patch to review:
diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index 5e899aba1ce..a06fc936163 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -1122,7 +1122,7 @@ def _run_userscript(self, cmd, *args, verbose=False):
         try:
             userscripts.run_async(tab, cmd, *args, win_id=self._win_id,
                                   env=env, verbose=verbose)
-        except userscripts.UnsupportedError as e:
+        except (userscripts.UnsupportedError, userscripts.NotFoundError) as e:
             raise cmdexc.CommandError(e)
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py
index 4601ed5288f..1cb4f14f494 100644
--- a/qutebrowser/commands/userscripts.py
+++ b/qutebrowser/commands/userscripts.py
@@ -315,6 +315,27 @@ def prepare_run(self, *args, **kwargs):
             return
 
 
+class NotFoundError(Exception):
+
+    """Raised when spawning a userscript that doesn't exist.
+
+    Attributes:
+        script_name: name of the userscript as called
+        paths: path names that were searched for the userscript
+        message: descriptive message string of the error
+    """
+
+    def __init__(self, script_name, paths=[]):
+        self.script_name = script_name
+        self.paths = paths
+        self.message = "Userscript '" + script_name + "' not found"
+        if paths:
+            self.message += " in userscript directory '" + ("' or '".join(paths)) + "'"
+
+    def __str__(self):
+        return self.message
+
+
 class UnsupportedError(Exception):
 
     """Raised when userscripts aren't supported on this platform."""
@@ -375,6 +396,13 @@ def run_async(tab, cmd, *args, win_id, env, verbose=False):
         if not os.path.exists(cmd_path):
             cmd_path = os.path.join(standarddir.system_data(),
                                     "userscripts", cmd)
+            if not os.path.exists(cmd_path):
+                raise NotFoundError(cmd, [
+                    os.path.join(standarddir.data(), "userscripts"),
+                    os.path.join(standarddir.system_data(), "userscripts"),
+                ])
+    elif not os.path.exists(cmd_path):
+        raise NotFoundError(cmd_path)
     log.misc.debug("Userscript to run: {}".format(cmd_path))
 
     runner.finished.connect(commandrunner.deleteLater)
diff --git a/tests/end2end/features/spawn.feature b/tests/end2end/features/spawn.feature
index 5cdeac0293e..341e6c921b4 100644
--- a/tests/end2end/features/spawn.feature
+++ b/tests/end2end/features/spawn.feature
@@ -10,7 +10,11 @@ Feature: :spawn
 
     Scenario: Starting a userscript which doesn't exist
         When I run :spawn -u this_does_not_exist
-        Then the error "Error while spawning userscript: The process failed to start." should be shown
+        Then regex "Userscript 'this_does_not_exist' not found in userscript directory '(.*)'( or '(.*)')*" should be logged
+
+    Scenario: Starting a userscript with absoloute path which doesn't exist
+        When I run :spawn -u /this_does_not_exist
+        Then the error "Userscript '/this_does_not_exist' not found" should be shown
 
     # https://github.com/The-Compiler/qutebrowser/issues/1614
     @posix


Code review comments:
diff_hunk: @@ -1122,7 +1122,7 @@ def _run_userscript(self, cmd, *args, verbose=False):
         try:
             userscripts.run_async(tab, cmd, *args, win_id=self._win_id,
                                   env=env, verbose=verbose)
-        except userscripts.UnsupportedError as e:
+        except (userscripts.UnsupportedError, userscripts.NotFoundError) as e:
path: qutebrowser/browser/commands.py
text: @author:
@user1-Compiler Is this style okay or would you prefer a common userscript exception base?


@user2:
It's fine, but I'd still prefer having an exception base because as soon as a third exception might be added in the future, that'll definitely be cleaner.


@author:
I'll add it then, better sooner than later