Refactor LongQueryError and EnvironmentError into KnownError

- Also generalize catch on histcategory sql query to fix #4687

7 files changed, 37 insertions, 41 deletions

diff --git a/qutebrowser/app.py b/qutebrowser/app.py
index 5c347fa86..d8b975ce3 100644
--- a/qutebrowser/app.py
+++ b/qutebrowser/app.py
@@ -451,7 +451,7 @@ def _init_modules(args, crash_handler):
 
         log.init.debug("Initializing web history...")
         history.init(q_app)
-    except sql.SqlEnvironmentError as e:
+    except sql.SqlKnownError as e:
         error.handle_fatal_exc(e, args, 'Error initializing SQL',
                                pre_text='Error initializing SQL')
         sys.exit(usertypes.Exit.err_init)
diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py
index 2480f7a7a..6b5a22f5a 100644
--- a/qutebrowser/browser/history.py
+++ b/qutebrowser/browser/history.py
@@ -195,7 +195,7 @@ class WebHistory(sql.SqlTable):
     def _handle_sql_errors(self):
         try:
             yield
-        except sql.SqlEnvironmentError as e:
+        except sql.SqlKnownError as e:
             message.error("Failed to write history: {}".format(e.text()))
 
     def _is_excluded(self, url):
diff --git a/qutebrowser/completion/models/histcategory.py b/qutebrowser/completion/models/histcategory.py
index 175b458fd..d53a899c7 100644
--- a/qutebrowser/completion/models/histcategory.py
+++ b/qutebrowser/completion/models/histcategory.py
@@ -85,28 +85,30 @@ class HistoryCategory(QSqlQueryModel):
         timefmt = ("strftime('{}', last_atime, 'unixepoch', 'localtime')"
                    .format(timestamp_format.replace("'", "`")))
 
-        if not self._query or len(words) != len(self._query.bound_values()):
-            # if the number of words changed, we need to generate a new query
-            # otherwise, we can reuse the prepared query for performance
-            try:
+        try:
+            if (not self._query or
+                    len(words) != len(self._query.bound_values())):
+                # if the number of words changed, we need to generate a new
+                # query otherwise, we can reuse the prepared query for
+                # performance
                 self._query = sql.Query(' '.join([
                     "SELECT url, title, {}".format(timefmt),
                     "FROM CompletionHistory",
-                    # the incoming pattern will have literal % and _ escaped
-                    # we need to tell sql to treat '\' as an escape character
+                    # the incoming pattern will have literal % and _ escaped we
+                    # need to tell sql to treat '\' as an escape character
                     'WHERE ({})'.format(where_clause),
                     self._atime_expr(),
                     "ORDER BY last_atime DESC",
                 ]), forward_only=False)
-            except sql.SqlLongQueryError as e:
-                # Sometimes, the query we built up was invalid, for example,
-                # due to a large amount of words.
-                message.error("Error building SQL Query: {}".format(e.text()))
-                return
-
-        with debug.log_time('sql', 'Running completion query'):
-            self._query.run(**{
-                str(i): w for i, w in enumerate(words)})
+
+            with debug.log_time('sql', 'Running completion query'):
+                self._query.run(**{
+                    str(i): w for i, w in enumerate(words)})
+        except sql.SqlKnownError as e:
+            # Sometimes, the query we built up was invalid, for example,
+            # due to a large amount of words.
+            message.error("Error with SQL Query: {}".format(e.text()))
+            return
         self.setQuery(self._query.query)
 
     def removeRows(self, row, _count, _parent=None):
diff --git a/qutebrowser/misc/sql.py b/qutebrowser/misc/sql.py
index dd33621f6..72474a9f4 100644
--- a/qutebrowser/misc/sql.py
+++ b/qutebrowser/misc/sql.py
@@ -65,7 +65,7 @@ class SqlError(Exception):
             return self.error.databaseText()
 
 
-class SqlEnvironmentError(SqlError):
+class SqlKnownError(SqlError):
 
     """Raised on an error interacting with the SQL database.
 
@@ -74,11 +74,6 @@ class SqlEnvironmentError(SqlError):
     """
 
 
-class SqlLongQueryError(SqlError):
-
-    """Raised when a query is too long."""
-
-
 class SqlBugError(SqlError):
 
     """Raised on an error interacting with the SQL database.
@@ -88,7 +83,7 @@ class SqlBugError(SqlError):
 
 
 def raise_sqlite_error(msg, error):
-    """Raise either a SqlBugError or SqlEnvironmentError."""
+    """Raise either a SqlBugError or SqlKnownError."""
     error_code = error.nativeErrorCode()
     database_text = error.databaseText()
     driver_text = error.driverText()
@@ -116,12 +111,12 @@ def raise_sqlite_error(msg, error):
                    driver_text == "Error opening database" and
                    database_text == "out of memory")
 
-    if error_code in environmental_errors or qtbug_70506:
-        raise SqlEnvironmentError(msg, error)
-    if (error_code == SqliteErrorCode.ERROR and
-            driver_text == "Unable to execute statement" and
-            database_text.startswith("Expression tree is too large")):
-        raise SqlLongQueryError(msg, error)
+    if ((error_code in environmental_errors or qtbug_70506) or
+            (error_code == SqliteErrorCode.ERROR and
+             driver_text == "Unable to execute statement" and
+             database_text.startswith("Expression tree is too large"))):
+        raise SqlKnownError(msg, error)
+
     raise SqlBugError(msg, error)
 
 
@@ -129,8 +124,8 @@ def init(db_path):
     """Initialize the SQL database connection."""
     database = QSqlDatabase.addDatabase('QSQLITE')
     if not database.isValid():
-        raise SqlEnvironmentError('Failed to add database. Are sqlite and Qt '
-                                  'sqlite support installed?')
+        raise SqlKnownError('Failed to add database. Are sqlite and Qt '
+                            'sqlite support installed?')
     database.setDatabaseName(db_path)
     if not database.open():
         error = database.lastError()
@@ -158,7 +153,7 @@ def version():
             close()
             return ver
         return Query("select sqlite_version()").run().value()
-    except SqlEnvironmentError as e:
+    except SqlKnownError as e:
         return 'UNAVAILABLE ({})'.format(e)
 
 
diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py
index 3d0f9f0bf..03b3292b6 100644
--- a/tests/unit/browser/test_history.py
+++ b/tests/unit/browser/test_history.py
@@ -189,9 +189,8 @@ class TestAdd:
                    environmental, completion):
         def raise_error(url, replace=False):
             if environmental:
-                raise sql.SqlEnvironmentError("Error message")
-            else:
-                raise sql.SqlBugError("Error message")
+                raise sql.SqlKnownError("Error message")
+            raise sql.SqlBugError("Error message")
 
         if completion:
             monkeypatch.setattr(web_history.completion, 'insert', raise_error)
diff --git a/tests/unit/completion/test_histcategory.py b/tests/unit/completion/test_histcategory.py
index 7dfda3ea6..b6db66c75 100644
--- a/tests/unit/completion/test_histcategory.py
+++ b/tests/unit/completion/test_histcategory.py
@@ -144,7 +144,7 @@ def test_set_pattern_long(hist, message_mock, caplog):
         # pylint: enable=bad-builtin
     msg = message_mock.getmsg(usertypes.MessageLevel.error)
     assert msg.text.startswith(
-        "Error building SQL Query: Expression tree is too large")
+        "Error with SQL Query: Expression tree is too large")
 
 
 @pytest.mark.parametrize('max_items, before, after', [
diff --git a/tests/unit/misc/test_sql.py b/tests/unit/misc/test_sql.py
index 6b1ee6c9e..00b4c9aa9 100644
--- a/tests/unit/misc/test_sql.py
+++ b/tests/unit/misc/test_sql.py
@@ -29,7 +29,7 @@ from qutebrowser.misc import sql
 pytestmark = pytest.mark.usefixtures('init_sql')
 
 
-@pytest.mark.parametrize('klass', [sql.SqlEnvironmentError, sql.SqlBugError])
+@pytest.mark.parametrize('klass', [sql.SqlKnownError, sql.SqlBugError])
 def test_sqlerror(klass):
     text = "Hello World"
     err = klass(text)
@@ -40,7 +40,7 @@ def test_sqlerror(klass):
 class TestSqlError:
 
     @pytest.mark.parametrize('error_code, exception', [
-        (sql.SqliteErrorCode.BUSY, sql.SqlEnvironmentError),
+        (sql.SqliteErrorCode.BUSY, sql.SqlKnownError),
         (sql.SqliteErrorCode.CONSTRAINT, sql.SqlBugError),
     ])
     def test_environmental(self, error_code, exception):
@@ -59,7 +59,7 @@ class TestSqlError:
                             "out of memory",
                             QSqlError.UnknownError,
                             sql.SqliteErrorCode.UNKNOWN)
-        with pytest.raises(sql.SqlEnvironmentError):
+        with pytest.raises(sql.SqlKnownError):
             sql.raise_sqlite_error("Message", sql_err)
 
     def test_logging(self, caplog):
@@ -77,7 +77,7 @@ class TestSqlError:
         assert caplog.messages == expected
 
     @pytest.mark.parametrize('klass',
-                             [sql.SqlEnvironmentError, sql.SqlBugError])
+                             [sql.SqlKnownError, sql.SqlBugError])
     def test_text(self, klass):
         sql_err = QSqlError("driver text", "db text")
         err = klass("Message", sql_err)