diff --git a/qutebrowser/app.py b/qutebrowser/app.py index ed2a85eed..8d3a11656 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -449,13 +449,10 @@ def _init_modules(args, crash_handler): log.init.debug("Initializing web history...") history.init(qApp) - except sql.SqlError as e: - if e.environmental: - error.handle_fatal_exc(e, args, 'Error initializing SQL', - pre_text='Error initializing SQL') - sys.exit(usertypes.Exit.err_init) - else: - raise + except sql.SqlEnvironmentError as e: + error.handle_fatal_exc(e, args, 'Error initializing SQL', + pre_text='Error initializing SQL') + sys.exit(usertypes.Exit.err_init) log.init.debug("Initializing completion...") completiondelegate.init() diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index 6b34a92b6..7d2ea10a4 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -145,11 +145,8 @@ class WebHistory(sql.SqlTable): def _handle_sql_errors(self): try: yield - except sql.SqlError as e: - if e.environmental: - message.error("Failed to write history: {}".format(e.text())) - else: - raise + except sql.SqlEnvironmentError as e: + message.error("Failed to write history: {}".format(e.text())) def _rebuild_completion(self): data = {'url': [], 'title': [], 'last_atime': []} diff --git a/qutebrowser/misc/sql.py b/qutebrowser/misc/sql.py index e525081a8..c93ee9b61 100644 --- a/qutebrowser/misc/sql.py +++ b/qutebrowser/misc/sql.py @@ -27,95 +27,104 @@ from PyQt5.QtSql import QSqlDatabase, QSqlQuery, QSqlError from qutebrowser.utils import log, debug -class SqlError(Exception): +class SqlEnvironmentError(Exception): """Raised on an error interacting with the SQL database. - Attributes: - environmental: Whether the error is likely caused by the environment - and not a qutebrowser bug. + This is raised for errors resulting from the environment, where qutebrowser + isn't to blame. """ - def __init__(self, msg, environmental=False): - super().__init__(msg) - self.environmental = environmental - def text(self): - """Get a short text to display.""" return str(self) -class SqliteError(SqlError): +class SqlBugError(Exception): - """A SQL error with a QSqlError available. + """Raised on an error interacting with the SQL database. - Attributes: - error: The QSqlError object. + This is raised for errors resulting from a qutebrowser bug. + """ + + def text(self): + return str(self) + + +class SqliteEnvironmentError(SqlEnvironmentError): + + """A sqlite error which is caused by the environment. + + This is raised in conditions like a full disk or I/O errors, where + qutebrowser isn't to blame. """ def __init__(self, msg, error): super().__init__(msg) self.error = error - log.sql.debug("SQL error:") - log.sql.debug("type: {}".format( - debug.qenum_key(QSqlError, error.type()))) - log.sql.debug("database text: {}".format(error.databaseText())) - log.sql.debug("driver text: {}".format(error.driverText())) - log.sql.debug("error code: {}".format(error.nativeErrorCode())) + def text(self): + return self.error.databaseText() - # https://sqlite.org/rescode.html - # https://github.com/qutebrowser/qutebrowser/issues/2930 - # https://github.com/qutebrowser/qutebrowser/issues/3004 - environmental_errors = [ - '5', # SQLITE_BUSY ("database is locked") - '8', # SQLITE_READONLY ("attempt to write a readonly database") - '10', # SQLITE_IOERR ("disk I/O error") - '11', # SQLITE_CORRUPT ("database disk image is malformed") - '13', # SQLITE_FULL ("database or disk is full") - '14', # SQLITE_CANTOPEN ("unable to open database file") - ] - # At least in init(), we can get errors like this: - # > type: ConnectionError - # > database text: out of memory - # > driver text: Error opening database - # > error code: -1 - environmental_strings = [ - "out of memory", - ] - errcode = error.nativeErrorCode() - self.environmental = ( - errcode in environmental_errors or - (errcode == -1 and error.databaseText() in environmental_strings)) + +class SqliteBugError(SqlBugError): + + """A sqlite error which is caused by a bug in qutebrowser.""" + + def __init__(self, msg, error): + super().__init__(msg) + self.error = error def text(self): return self.error.databaseText() - @classmethod - def from_query(cls, what, query, error): - """Construct an error from a failed query. - Arguments: - what: What we were doing when the error happened. - query: The query which was executed. - error: The QSqlError object. - """ - msg = 'Failed to {} query "{}": "{}"'.format(what, query, error.text()) - return cls(msg, error) +def raise_sqlite_error(msg, error): + """Raise either a SqliteBugError or SqliteEnvironmentError.""" + log.sql.debug("SQL error:") + log.sql.debug("type: {}".format( + debug.qenum_key(QSqlError, error.type()))) + log.sql.debug("database text: {}".format(error.databaseText())) + log.sql.debug("driver text: {}".format(error.driverText())) + log.sql.debug("error code: {}".format(error.nativeErrorCode())) + # https://sqlite.org/rescode.html + # https://github.com/qutebrowser/qutebrowser/issues/2930 + # https://github.com/qutebrowser/qutebrowser/issues/3004 + environmental_errors = [ + '5', # SQLITE_BUSY ("database is locked") + '8', # SQLITE_READONLY ("attempt to write a readonly database") + '10', # SQLITE_IOERR ("disk I/O error") + '11', # SQLITE_CORRUPT ("database disk image is malformed") + '13', # SQLITE_FULL ("database or disk is full") + '14', # SQLITE_CANTOPEN ("unable to open database file") + ] + # At least in init(), we can get errors like this: + # > type: ConnectionError + # > database text: out of memory + # > driver text: Error opening database + # > error code: -1 + environmental_strings = [ + "out of memory", + ] + errcode = error.nativeErrorCode() + if (errcode in environmental_errors or + (errcode == -1 and error.databaseText() in environmental_strings)): + raise SqliteEnvironmentError(msg, error) + else: + raise SqliteBugError(msg, error) def init(db_path): """Initialize the SQL database connection.""" database = QSqlDatabase.addDatabase('QSQLITE') if not database.isValid(): - raise SqlError('Failed to add database. ' - 'Are sqlite and Qt sqlite support installed?', - environmental=True) + raise SqlEnvironmentError('Failed to add database. Are sqlite and Qt ' + 'sqlite support installed?') database.setDatabaseName(db_path) if not database.open(): error = database.lastError() - raise SqliteError("Failed to open sqlite database at {}: {}" - .format(db_path, error.text()), error) + msg = "Failed to open sqlite database at {}: {}".format(db_path, + error.text()) + raise_sqlite_error(msg, error) # Enable write-ahead-logging and reduce disk write frequency # see https://sqlite.org/pragma.html and issues #2930 and #3507 @@ -137,7 +146,7 @@ def version(): close() return ver return Query("select sqlite_version()").run().value() - except SqlError as e: + except SqlEnvironmentError as e: return 'UNAVAILABLE ({})'.format(e) @@ -162,7 +171,7 @@ class Query: def __iter__(self): if not self.query.isActive(): - raise SqlError("Cannot iterate inactive query") + raise SqlBugError("Cannot iterate inactive query") rec = self.query.record() fields = [rec.fieldName(i) for i in range(rec.count())] rowtype = collections.namedtuple('ResultRow', fields) @@ -173,8 +182,11 @@ class Query: def _check_ok(self, step, ok): if not ok: - raise SqliteError.from_query(step, self.query.lastQuery(), - self.query.lastError()) + query = self.query.lastQuery() + error = self.query.lastError() + msg = 'Failed to {} query "{}": "{}"'.format(step, query, + error.text()) + raise_sqlite_error(msg, error) def run(self, **values): """Execute the prepared query.""" @@ -185,7 +197,7 @@ class Query: self.query.bindValue(':{}'.format(key), val) log.sql.debug('query bindings: {}'.format(self.bound_values())) if any(val is None for val in self.bound_values().values()): - raise SqlError("Missing bound values!") + raise SqlBugError("Missing bound values!") ok = self.query.exec_() self._check_ok('exec', ok) @@ -200,7 +212,7 @@ class Query: for key, val in values.items(): self.query.bindValue(':{}'.format(key), val) if any(val is None for val in self.bound_values().values()): - raise SqlError("Missing bound values!") + raise SqlBugError("Missing bound values!") db = QSqlDatabase.database() ok = db.transaction() @@ -209,7 +221,7 @@ class Query: ok = self.query.execBatch() try: self._check_ok('execBatch', ok) - except SqliteError: + except (SqliteBugError, SqliteEnvironmentError): # Not checking the return value here, as we're failing anyways... db.rollback() raise @@ -220,7 +232,7 @@ class Query: def value(self): """Return the result of a single-value query (e.g. an EXISTS).""" if not self.query.next(): - raise SqlError("No result for single-result query") + raise SqlBugError("No result for single-result query") return self.query.record().value(0) def rows_affected(self): diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index 6eaa399e7..d0b1134c8 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -190,7 +190,10 @@ class TestAdd: def test_error(self, monkeypatch, hist, message_mock, caplog, environmental, completion): def raise_error(url, replace=False): - raise sql.SqlError("Error message", environmental=environmental) + if environmental: + raise sql.SqlEnvironmentError("Error message") + else: + raise sql.SqlBugError("Error message") if completion: monkeypatch.setattr(hist.completion, 'insert', raise_error) @@ -203,7 +206,7 @@ class TestAdd: msg = message_mock.getmsg(usertypes.MessageLevel.error) assert msg.text == "Failed to write history: Error message" else: - with pytest.raises(sql.SqlError): + with pytest.raises(sql.SqlBugError): hist.add_url(QUrl('https://www.example.org/')) @pytest.mark.parametrize('level, url, req_url, expected', [ diff --git a/tests/unit/misc/test_sql.py b/tests/unit/misc/test_sql.py index 67ca5e5c1..28aa912e2 100644 --- a/tests/unit/misc/test_sql.py +++ b/tests/unit/misc/test_sql.py @@ -29,30 +29,32 @@ from qutebrowser.misc import sql pytestmark = pytest.mark.usefixtures('init_sql') -def test_sqlerror(): +@pytest.mark.parametrize('klass', [sql.SqlEnvironmentError, sql.SqlBugError]) +def test_sqlerror(klass): text = "Hello World" - err = sql.SqlError(text, environmental=True) + err = klass(text) assert str(err) == text assert err.text() == text - assert err.environmental class TestSqliteError: - @pytest.mark.parametrize('error_code, environmental', [ - ('5', True), # SQLITE_BUSY - ('19', False), # SQLITE_CONSTRAINT + @pytest.mark.parametrize('error_code, exception', [ + ('5', sql.SqliteEnvironmentError), # SQLITE_BUSY + ('19', sql.SqliteBugError), # SQLITE_CONSTRAINT ]) - def test_environmental(self, error_code, environmental): + def test_environmental(self, error_code, exception): sql_err = QSqlError("driver text", "db text", QSqlError.UnknownError, error_code) - err = sql.SqliteError("Message", sql_err) - assert err.environmental == environmental + with pytest.raises(exception): + sql.raise_sqlite_error("Message", sql_err) def test_logging(self, caplog): sql_err = QSqlError("driver text", "db text", QSqlError.UnknownError, '23') - sql.SqliteError("Message", sql_err) + with pytest.raises(sql.SqliteBugError): + sql.raise_sqlite_error("Message", sql_err) + lines = [r.message for r in caplog.records] expected = ['SQL error:', 'type: UnknownError', @@ -62,21 +64,19 @@ class TestSqliteError: assert lines == expected - def test_from_query(self): - sql_err = QSqlError("driver text", "db text") - err = sql.SqliteError.from_query( - what='test', query='SELECT * from foo;', error=sql_err) - expected = ('Failed to test query "SELECT * from foo;": ' - '"db text driver text"') - assert str(err) == expected + @pytest.mark.parametrize('base, sub', [ + (sql.SqlEnvironmentError, sql.SqliteEnvironmentError), + (sql.SqlBugError, sql.SqliteBugError), + ]) + def test_subclass(self, base, sub): + with pytest.raises(base): + raise sub("text", QSqlError()) - def test_subclass(self): - with pytest.raises(sql.SqlError): - raise sql.SqliteError("text", QSqlError()) - - def test_text(self): + @pytest.mark.parametrize('klass', + [sql.SqliteEnvironmentError, sql.SqliteBugError]) + def test_text(self, klass): sql_err = QSqlError("driver text", "db text") - err = sql.SqliteError("Message", sql_err) + err = klass("Message", sql_err) assert err.text() == "db text" @@ -103,7 +103,7 @@ def test_insert_replace(qtbot): table.insert({'name': 'one', 'val': 11, 'lucky': True}, replace=True) assert list(table) == [('one', 11, True)] - with pytest.raises(sql.SqlError): + with pytest.raises(sql.SqliteBugError): table.insert({'name': 'one', 'val': 11, 'lucky': True}, replace=False) @@ -139,7 +139,7 @@ def test_insert_batch_replace(qtbot): ('one', 11, True), ('nine', 19, True)] - with pytest.raises(sql.SqlError): + with pytest.raises(sql.SqliteBugError): table.insert_batch({'name': ['one', 'nine'], 'val': [11, 19], 'lucky': [True, True]}) @@ -236,10 +236,12 @@ def test_version(): class TestSqlQuery: def test_prepare_error(self): - with pytest.raises(sql.SqlError) as excinfo: + with pytest.raises(sql.SqliteBugError) as excinfo: sql.Query('invalid') - msg = excinfo.value.error.databaseText() - assert msg == 'near "invalid": syntax error' + + expected = ('Failed to prepare query "invalid": "near "invalid": ' + 'syntax error Unable to execute statement"') + assert str(excinfo.value) == expected @pytest.mark.parametrize('forward_only', [True, False]) def test_forward_only(self, forward_only): @@ -248,7 +250,7 @@ class TestSqlQuery: def test_iter_inactive(self): q = sql.Query('SELECT 0') - with pytest.raises(sql.SqlError, + with pytest.raises(sql.SqlBugError, match='Cannot iterate inactive query'): next(iter(q)) @@ -277,7 +279,7 @@ class TestSqlQuery: def test_run_missing_binding(self): q = sql.Query('SELECT :answer') - with pytest.raises(sql.SqlError, match='Missing bound values!'): + with pytest.raises(sql.SqlBugError, match='Missing bound values!'): q.run() def test_run_batch(self): @@ -287,13 +289,13 @@ class TestSqlQuery: def test_run_batch_missing_binding(self): q = sql.Query('SELECT :answer') - with pytest.raises(sql.SqlError, match='Missing bound values!'): + with pytest.raises(sql.SqlBugError, match='Missing bound values!'): q.run_batch(values={}) def test_value_missing(self): q = sql.Query('SELECT 0 WHERE 0') q.run() - with pytest.raises(sql.SqlError, + with pytest.raises(sql.SqlBugError, match='No result for single-result query'): q.value()