Use exceptions for command errors

TODO

@@ -102,7 +102,6 @@ Style
 - rename commands
 - reorder config #2
 - rework exception hierarchy for config (common base exception)
-- use exceptions for error messages in commands
 
 Major features
 ==============

qutebrowser/app.py

@@ -48,6 +48,7 @@ import qutebrowser.config.config as config
 import qutebrowser.network.qutescheme as qutescheme
 import qutebrowser.config.websettings as websettings
 import qutebrowser.network.proxy as proxy
+import qutebrowser.utils.message as message
 from qutebrowser.network.networkmanager import NetworkManager
 from qutebrowser.config.config import ConfigManager
 from qutebrowser.keyinput.modeman import ModeManager
@@ -56,6 +57,7 @@ from qutebrowser.widgets.crash import CrashDialog
 from qutebrowser.keyinput.modeparsers import NormalKeyParser, HintKeyParser
 from qutebrowser.keyinput.keyparser import PassthroughKeyParser
 from qutebrowser.commands.managers import CommandManager, SearchManager
+from qutebrowser.commands.exceptions import CommandError
 from qutebrowser.config.iniparsers import ReadWriteConfigParser
 from qutebrowser.config.lineparser import LineConfigParser
 from qutebrowser.browser.cookies import CookieJar
@@ -581,7 +583,10 @@ class QuteBrowser(QApplication):
         else:
             obj = dotted_getattr(self, instance)
         handler = getattr(obj, func)
+        try:
             if count is not None:
                 handler(*args, count=count)
             else:
                 handler(*args)
+        except CommandError as e:
+            message.error(e)

qutebrowser/browser/curcommand.py

@@ -35,6 +35,7 @@ import qutebrowser.utils.webelem as webelem
 import qutebrowser.config.config as config
 import qutebrowser.browser.hints as hints
 from qutebrowser.utils.misc import shell_escape
+from qutebrowser.commands.exceptions import CommandError
 
 
 class CurCommandDispatcher(QObject):
@@ -85,8 +86,7 @@ class CurCommandDispatcher(QObject):
         widget = self._tabs.currentWidget()
         frame = widget.page_.currentFrame()
         if frame is None:
-            message.error("No frame focused!")
+            raise CommandError("No frame focused!")
-            return
         widget.hintmanager.follow_prevnext(frame, widget.url(), prev, newtab)
 
     @cmdutils.register(instance='mainwindow.tabs.cur', name='open',
@@ -193,8 +193,7 @@ class CurCommandDispatcher(QObject):
             count: How many pages to go back.
         """
         for _ in range(count):
-            if not self._tabs.currentWidget().go_back():
+            self._tabs.currentWidget().go_back()
-                break
 
     @cmdutils.register(instance='mainwindow.tabs.cur')
     def forward(self, count=1):
@@ -206,8 +205,7 @@ class CurCommandDispatcher(QObject):
             count: How many pages to go forward.
         """
         for _ in range(count):
-            if not self._tabs.currentWidget().go_forward():
+            self._tabs.currentWidget().go_forward()
-                break
 
     @cmdutils.register(instance='mainwindow.tabs.cur')
     def hint(self, groupstr='all', targetstr='normal'):
@@ -222,18 +220,15 @@ class CurCommandDispatcher(QObject):
         widget = self._tabs.currentWidget()
         frame = widget.page_.mainFrame()
         if frame is None:
-            message.error("No frame focused!")
+            raise CommandError("No frame focused!")
-            return
         try:
             group = getattr(webelem.Group, groupstr)
         except AttributeError:
-            message.error("Unknown hinting group {}!".format(groupstr))
+            raise CommandError("Unknown hinting group {}!".format(groupstr))
-            return
         try:
             target = getattr(hints.Target, targetstr)
         except AttributeError:
-            message.error("Unknown hinting target {}!".format(targetstr))
+            raise CommandError("Unknown hinting target {}!".format(targetstr))
-            return
         widget.hintmanager.start(frame, widget.url(), group, target)
 
     @cmdutils.register(instance='mainwindow.tabs.cur', hide=True)
@@ -403,7 +398,7 @@ class CurCommandDispatcher(QObject):
         try:
             level = cmdutils.arg_or_count(zoom, count, default=100)
         except ValueError as e:
-            message.error(e)
+            raise CommandError(e)
         tab = self._tabs.currentWidget()
         tab.zoom_perc(level)
 
@@ -447,8 +442,7 @@ class CurCommandDispatcher(QObject):
         elem = frame.findFirstElement(webelem.SELECTORS[
             webelem.Group.editable_focused])
         if elem.isNull():
-            message.error("No editable element focused!")
+            raise CommandError("No editable element focused!")
-            return
         oshandle, filename = mkstemp(text=True)
         text = elem.evaluateJavaScript('this.value')
         if text:
@@ -470,7 +464,7 @@ class CurCommandDispatcher(QObject):
         try:
             os.remove(filename)
         except PermissionError:
-            message.error("Failed to delete tempfile...")
+            raise CommandError("Failed to delete tempfile...")
 
     def on_editor_closed(self, elem, oshandle, filename, exitcode,
                          exitstatus):
@@ -480,15 +474,13 @@ class CurCommandDispatcher(QObject):
         """
         logging.debug("Editor closed")
         if exitcode != 0:
-            message.error("Editor did quit abnormally (status {})!".format(
+            raise CommandError("Editor did quit abnormally (status "
-                exitcode))
+                               "{})!".format(exitcode))
-            return
         if exitstatus != QProcess.NormalExit:
             # No error here, since we already handle this in on_editor_error
             return
         if elem.isNull():
-            message.error("Element vanished while editing!")
+            raise CommandError("Element vanished while editing!")
-            return
         with open(filename, 'r', encoding='utf-8') as f:
             text = ''.join(f.readlines())
             text = webelem.javascript_escape(text)
@@ -508,5 +500,6 @@ class CurCommandDispatcher(QObject):
                                  "from the process."),
             QProcess.UnknownError: "An unknown error occurred.",
         }
-        message.error("Error while calling editor: {}".format(messages[error]))
         self._editor_cleanup(oshandle, filename)
+        raise CommandError("Error while calling editor: {}".format(
+            messages[error]))

qutebrowser/browser/hints.py

@@ -30,6 +30,7 @@ import qutebrowser.keyinput.modeman as modeman
 import qutebrowser.utils.message as message
 import qutebrowser.utils.url as urlutils
 import qutebrowser.utils.webelem as webelem
+from qutebrowser.commands.exceptions import CommandError
 from qutebrowser.utils.usertypes import enum
 
 
@@ -371,14 +372,12 @@ class HintManager(QObject):
         """
         elem = self._find_prevnext(frame, prev)
         if elem is None:
-            message.error("No {} links found!".format("prev" if prev
+            raise CommandError("No {} links found!".format(
-                                                      else "forward"))
+                "prev" if prev else "forward"))
-            return
         link = self._resolve_link(elem, baseurl)
         if link is None:
-            message.error("No {} links found!".format("prev" if prev
+            raise CommandError("No {} links found!".format(
-                                                      else "forward"))
+                "prev" if prev else "forward"))
-            return
         self.openurl.emit(link, newtab)
 
     def start(self, mainframe, baseurl, group=webelem.Group.all,
@@ -406,8 +405,7 @@ class HintManager(QObject):
         visible_elems = [e for e in elems if filterfunc(e) and
                          webelem.is_visible(e, mainframe)]
         if not visible_elems:
-            message.error("No elements found.")
+            raise CommandError("No elements found.")
-            return
         self._target = target
         self._baseurl = baseurl
         self._frames = webelem.get_child_frames(mainframe)
@@ -498,8 +496,7 @@ class HintManager(QObject):
         elif self._target in link_handlers:
             link = self._resolve_link(elem)
             if link is None:
-                message.error("No suitable link found for this element.")
+                raise CommandError("No suitable link found for this element.")
-                return
             link_handlers[self._target](link)
         else:
             raise ValueError("No suitable handler found!")
@@ -509,8 +506,7 @@ class HintManager(QObject):
     def follow_hint(self):
         """Follow the currently selected hint."""
         if not self._to_follow:
-            message.error("No hint to follow")
+            raise CommandError("No hint to follow")
-            return
         self.fire(self._to_follow, force=True)
 
     @pyqtSlot('QSize')

qutebrowser/commands/_command.py

@@ -19,7 +19,7 @@
 
 import logging
 
-from qutebrowser.commands._exceptions import (ArgumentCountError,
+from qutebrowser.commands.exceptions import (ArgumentCountError,
                                              PrerequisitesError)
 
 from PyQt5.QtCore import pyqtSignal, QObject, QCoreApplication
@@ -114,6 +114,8 @@ class Command(QObject):
     def run(self, args=None, count=None):
         """Run the command.
 
+        Note we don't catch CommandError here as it might happen async.
+
         Args:
             args: Arguments to the command.
             count: Command repetition count.

qutebrowser/commands/exceptions.py

@@ -23,24 +23,31 @@ Defined here to avoid circular dependency hell.
 
 class CommandError(Exception):
 
-    """Common base class for all command exceptions."""
+    """Raised when a command encounters a error while running."""
+
+    pass
 
 
-class NoSuchCommandError(CommandError):
+class CommandMetaError(Exception):
+
+    """Common base class for exceptions occuring before a command is run."""
+
+
+class NoSuchCommandError(CommandMetaError):
 
     """Raised when a command wasn't found."""
 
     pass
 
 
-class ArgumentCountError(CommandError):
+class ArgumentCountError(CommandMetaError):
 
     """Raised when a command was called with an invalid count of arguments."""
 
     pass
 
 
-class PrerequisitesError(CommandError):
+class PrerequisitesError(CommandMetaError):
 
     """Raised when a cmd can't be used because some prerequisites aren't met.
 

qutebrowser/commands/managers.py

@@ -25,7 +25,8 @@ from PyQt5.QtWebKitWidgets import QWebPage
 import qutebrowser.config.config as config
 import qutebrowser.commands.utils as cmdutils
 import qutebrowser.utils.message as message
-from qutebrowser.commands._exceptions import NoSuchCommandError, CommandError
+from qutebrowser.commands.exceptions import (NoSuchCommandError,
+                                             CommandMetaError, CommandError)
 from qutebrowser.utils.misc import safe_shlex_split
 
 
@@ -227,5 +228,5 @@ class CommandManager:
         """Run a command and display exceptions in the statusbar."""
         try:
             self.run(text, count)
-        except CommandError as e:
+        except (CommandMetaError, CommandError) as e:
-            message.error(str(e))
+            message.error(e)

qutebrowser/config/config.py

@@ -37,6 +37,7 @@ import qutebrowser.commands.utils as cmdutils
 import qutebrowser.utils.message as message
 from qutebrowser.config.iniparsers import ReadConfigParser
 from qutebrowser.config._conftypes import ValidationError
+from qutebrowser.commands.exceptions import CommandError
 
 
 def instance():
@@ -258,7 +259,7 @@ class ConfigManager(QObject):
         try:
             val = self.get(sectname, optname, transformed=False)
         except (NoOptionError, NoSectionError) as e:
-            message.error("get: {} - {}".format(e.__class__.__name__, e))
+            raise CommandError("get: {} - {}".format(e.__class__.__name__, e))
         else:
             message.info("{} {} = {}".format(sectname, optname, val))
 

qutebrowser/keyinput/keyparser.py

@@ -23,7 +23,8 @@ from qutebrowser.keyinput._basekeyparser import BaseKeyParser
 import qutebrowser.utils.message as message
 
 from qutebrowser.commands.managers import CommandManager
-from qutebrowser.commands._exceptions import ArgumentCountError, CommandError
+from qutebrowser.commands.exceptions import (
+    ArgumentCountError, CommandMetaError, CommandError)
 
 
 class CommandKeyParser(BaseKeyParser):
@@ -52,8 +53,8 @@ class CommandKeyParser(BaseKeyParser):
             logging.debug("Filling statusbar with partial command {}".format(
                 cmdstr))
             message.set_cmd_text(':{} '.format(cmdstr))
-        except CommandError as e:
+        except (CommandMetaError, CommandError) as e:
-            message.error(str(e))
+            message.error(e)
 
     def execute(self, cmdstr, _keytype, count=None):
         self._run_or_fill(cmdstr, count)

qutebrowser/network/networkmanager.py

@@ -63,6 +63,7 @@ class NetworkManager(QNetworkAccessManager):
         if config.get('network', 'ssl-strict'):
             return
         for err in errors:
+            # FIXME we might want to use warn here (non-fatal error)
             message.error('SSL error: {}'.format(err.errorString()))
         reply.ignoreSslErrors()
 

qutebrowser/utils/message.py

@@ -29,18 +29,21 @@ def instance():
 
 def error(message):
     """Display an error message in the statusbar."""
+    message = str(message)
     logging.error(message)
     instance().error.emit(message)
 
 
 def info(message):
     """Display a temporary info message in the statusbar."""
+    message = str(message)
     logging.info(message)
     instance().info.emit(message)
 
 
 def text(message):
     """Display a persistent message in the statusbar."""
+    message = str(message)
     logging.debug(message)
     instance().text.emit(message)
 

qutebrowser/widgets/_tabbedbrowser.py

@@ -32,6 +32,7 @@ from qutebrowser.widgets._tabwidget import TabWidget, EmptyTabIcon
 from qutebrowser.widgets.webview import WebView
 from qutebrowser.browser.signalfilter import SignalFilter
 from qutebrowser.browser.curcommand import CurCommandDispatcher
+from qutebrowser.commands.exceptions import CommandError
 
 
 class TabbedBrowser(TabWidget):
@@ -337,7 +338,7 @@ class TabbedBrowser(TabWidget):
         if self._url_stack:
             self.tabopen(self._url_stack.pop())
         else:
-            message.error("Nothing to undo!")
+            raise CommandError("Nothing to undo!")
 
     @cmdutils.register(instance='mainwindow.tabs', name='tabprev')
     def switch_prev(self, count=1):
@@ -354,7 +355,7 @@ class TabbedBrowser(TabWidget):
         elif config.get('tabbar', 'wrap'):
             self.setCurrentIndex(newidx % self.count())
         else:
-            message.error("First tab")
+            raise CommandError("First tab")
 
     @cmdutils.register(instance='mainwindow.tabs', name='tabnext')
     def switch_next(self, count=1):
@@ -371,7 +372,7 @@ class TabbedBrowser(TabWidget):
         elif config.get('tabbar', 'wrap'):
             self.setCurrentIndex(newidx % self.count())
         else:
-            message.error("Last tab")
+            raise CommandError("Last tab")
 
     @cmdutils.register(instance='mainwindow.tabs', nargs=(0, 1))
     def paste(self, sel=False, tab=False):
@@ -387,8 +388,7 @@ class TabbedBrowser(TabWidget):
         mode = QClipboard.Selection if sel else QClipboard.Clipboard
         url = clip.text(mode)
         if not url:
-            message.error("Clipboard is empty.")
+            raise CommandError("Clipboard is empty.")
-            return
         logging.debug("Clipboard contained: '{}'".format(url))
         if tab:
             self.tabopen(url)
@@ -417,13 +417,11 @@ class TabbedBrowser(TabWidget):
             idx = cmdutils.arg_or_count(index, count, default=1,
                                         countzero=self.count())
         except ValueError as e:
-            message.error(e)
+            raise CommandError(e)
-            return
         if 1 <= idx <= self.count():
             self.setCurrentIndex(idx - 1)
         else:
-            message.error("There's no tab with index {}!".format(idx))
+            raise CommandError("There's no tab with index {}!".format(idx))
-            return
 
     @cmdutils.register(instance='mainwindow.tabs')
     def tab_move(self, direction=None, count=None):
@@ -440,14 +438,12 @@ class TabbedBrowser(TabWidget):
             try:
                 new_idx = self._tab_move_relative(direction, count)
             except ValueError:
-                message.error("Count must be given for relative moving!")
+                raise CommandError("Count must be given for relative moving!")
-                return
         else:
-            message.error("Invalid direction '{}'!".format(direction))
+            raise CommandError("Invalid direction '{}'!".format(direction))
-            return
         if not 0 <= new_idx < self.count():
-            message.error("Can't move tab to position {}!".format(new_idx))
+            raise CommandError("Can't move tab to position {}!".format(
-            return
+                new_idx))
         tab = self.currentWidget()
         cur_idx = self.currentIndex()
         icon = self.tabIcon(cur_idx)
@@ -461,8 +457,7 @@ class TabbedBrowser(TabWidget):
         """Select the tab which was last focused."""
         idx = self.indexOf(self.last_focused)
         if idx == -1:
-            message.error("Last focused tab vanished!")
+            raise CommandError("Last focused tab vanished!")
-            return
         self.setCurrentIndex(idx)
 
     @pyqtSlot(str, str)

qutebrowser/widgets/webview.py

@@ -34,6 +34,7 @@ from qutebrowser.browser.webpage import BrowserPage
 from qutebrowser.browser.hints import HintManager
 from qutebrowser.utils.signals import SignalCache
 from qutebrowser.utils.usertypes import NeighborList, enum
+from qutebrowser.commands.exceptions import CommandError
 
 
 Target = enum('normal', 'tab', 'bgtab')
@@ -165,8 +166,7 @@ class WebView(QWebView):
         try:
             u = urlutils.fuzzy_url(url)
         except urlutils.SearchEngineError as e:
-            message.error(str(e))
+            raise CommandError(e)
-            return
         logging.debug("New title: {}".format(urlutils.urlstring(u)))
         self.titleChanged.emit(urlutils.urlstring(u))
         self.urlChanged.emit(urlutils.qurl(u))
@@ -194,30 +194,18 @@ class WebView(QWebView):
         self.zoom_perc(level, fuzzyval=False)
 
     def go_back(self):
-        """Go back a page in the history.
+        """Go back a page in the history."""
-
-        Return:
-            True if going back succeeded, False otherwise.
-        """
         if self.page_.history().canGoBack():
             self.back()
-            return True
         else:
-            message.error("At beginning of history.")
+            raise CommandError("At beginning of history.")
-            return False
 
     def go_forward(self):
-        """Go forward a page in the history.
+        """Go forward a page in the history."""
-
-        Return:
-            True if going forward succeeded, False otherwise.
-        """
         if self.page_.history().canGoForward():
             self.forward()
-            return True
         else:
-            message.error("At end of history.")
+            raise CommandError("At end of history.")
-            return False
 
     def shutdown(self, callback=None):
         """Shut down the tab cleanly and remove it.
@@ -385,11 +373,17 @@ class WebView(QWebView):
         """
         if e.button() == Qt.XButton1:
             # Back button on mice which have it.
+            try:
                 self.go_back()
+            except CommandError as ex:
+                message.error(ex)
             return super().mousePressEvent(e)
         elif e.button() == Qt.XButton2:
             # Forward button on mice which have it.
+            try:
                 self.go_forward()
+            except CommandError as ex:
+                message.error(ex)
             return super().mousePressEvent(e)
         pos = e.pos()
         frame = self.page_.frameAt(pos)