diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 9b5de8e2e..a64d2236e 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -439,13 +439,19 @@ def _init_modules(args, crash_handler): readline_bridge = readline.ReadlineBridge() objreg.register('readline-bridge', readline_bridge) - log.init.debug("Initializing sql...") try: + log.init.debug("Initializing sql...") sql.init(os.path.join(standarddir.data(), 'history.sqlite')) + + log.init.debug("Initializing web history...") + history.init(qApp) except sql.SqlError as e: - error.handle_fatal_exc(e, args, 'Error initializing SQL', - pre_text='Error initializing SQL') - sys.exit(usertypes.Exit.err_init) + 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 log.init.debug("Initializing completion...") completiondelegate.init() @@ -453,9 +459,6 @@ def _init_modules(args, crash_handler): log.init.debug("Initializing command history...") cmdhistory.init() - log.init.debug("Initializing web history...") - history.init(qApp) - log.init.debug("Initializing crashlog...") if not args.no_err_windows: crash_handler.handle_segfault() diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index 2c4c1f226..de9e9bb4f 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -190,15 +190,23 @@ class WebHistory(sql.SqlTable): return atime = int(atime) if (atime is not None) else int(time.time()) - self.insert({'url': self._format_url(url), - 'title': title, - 'atime': atime, - 'redirect': redirect}) - if not redirect: - self.completion.insert({'url': self._format_completion_url(url), - 'title': title, - 'last_atime': atime}, - replace=True) + + try: + self.insert({'url': self._format_url(url), + 'title': title, + 'atime': atime, + 'redirect': redirect}) + if not redirect: + self.completion.insert({ + 'url': self._format_completion_url(url), + 'title': title, + 'last_atime': atime + }, replace=True) + except sql.SqlError as e: + if e.environmental: + message.error("Failed to write history: {}".format(e.text())) + else: + raise def _parse_entry(self, line): """Parse a history line like '12345 http://example.com title'.""" diff --git a/qutebrowser/misc/sql.py b/qutebrowser/misc/sql.py index 24e2035e6..5b0d3361d 100644 --- a/qutebrowser/misc/sql.py +++ b/qutebrowser/misc/sql.py @@ -29,9 +29,66 @@ from qutebrowser.utils import log, debug class SqlError(Exception): - """Raised on an error interacting with the SQL database.""" + """Raised on an error interacting with the SQL database. - pass + Attributes: + environmental: Whether the error is likely caused by the environment + and not a qutebrowser bug. + """ + + 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): + + """A SQL error with a QSqlError available. + + Attributes: + error: The QSqlError object. + """ + + 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())) + + # https://sqlite.org/rescode.html + environmental_errors = [ + # SQLITE_LOCKED, + # https://github.com/qutebrowser/qutebrowser/issues/2930 + '9', + # SQLITE_FULL, + # https://github.com/qutebrowser/qutebrowser/issues/3004 + '13', + ] + self.environmental = error.nativeErrorCode() in environmental_errors + + 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 init(db_path): @@ -39,13 +96,13 @@ def init(db_path): database = QSqlDatabase.addDatabase('QSQLITE') if not database.isValid(): raise SqlError('Failed to add database. ' - 'Are sqlite and Qt sqlite support installed?') + 'Are sqlite and Qt sqlite support installed?', + environmental=True) database.setDatabaseName(db_path) if not database.open(): error = database.lastError() - _log_error(error) - raise SqlError("Failed to open sqlite database at {}: {}" - .format(db_path, error.text())) + raise SqliteError("Failed to open sqlite database at {}: {}" + .format(db_path, error.text()), error) def close(): @@ -66,28 +123,6 @@ def version(): return 'UNAVAILABLE ({})'.format(e) -def _log_error(error): - """Log informations about a SQL error to the debug log.""" - 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 _handle_query_error(what, query, error): - """Handle a sqlite error. - - Arguments: - what: What we were doing when the error happened. - query: The query which was executed. - error: The QSqlError object. - """ - _log_error(error) - msg = 'Failed to {} query "{}": "{}"'.format(what, query, error.text()) - raise SqlError(msg) - - class Query(QSqlQuery): """A prepared SQL Query.""" @@ -103,7 +138,7 @@ class Query(QSqlQuery): super().__init__(QSqlDatabase.database()) log.sql.debug('Preparing SQL query: "{}"'.format(querystr)) if not self.prepare(querystr): - _handle_query_error('prepare', querystr, self.lastError()) + raise SqliteError.from_query('prepare', querystr, self.lastError()) self.setForwardOnly(forward_only) def __iter__(self): @@ -124,7 +159,8 @@ class Query(QSqlQuery): self.bindValue(':{}'.format(key), val) log.sql.debug('query bindings: {}'.format(self.boundValues())) if not self.exec_(): - _handle_query_error('exec', self.lastQuery(), self.lastError()) + raise SqliteError.from_query('exec', self.lastQuery(), + self.lastError()) return self def value(self): @@ -250,7 +286,7 @@ class SqlTable(QObject): db = QSqlDatabase.database() db.transaction() if not q.execBatch(): - _handle_query_error('exec', q.lastQuery(), q.lastError()) + raise SqliteError.from_query('exec', q.lastQuery(), q.lastError()) db.commit() self.changed.emit() diff --git a/tests/unit/browser/test_history.py b/tests/unit/browser/test_history.py index 1454e259b..1b706331a 100644 --- a/tests/unit/browser/test_history.py +++ b/tests/unit/browser/test_history.py @@ -27,6 +27,7 @@ from PyQt5.QtCore import QUrl from qutebrowser.browser import history from qutebrowser.utils import objreg, urlutils, usertypes from qutebrowser.commands import cmdexc +from qutebrowser.misc import sql @pytest.fixture(autouse=True) @@ -178,6 +179,28 @@ def test_add_url_invalid(qtbot, hist, caplog): assert not list(hist.completion) +@pytest.mark.parametrize('environmental', [True, False]) +@pytest.mark.parametrize('completion', [True, False]) +def test_add_url_error(monkeypatch, hist, message_mock, caplog, + environmental, completion): + def raise_error(url, replace=False): + raise sql.SqlError("Error message", environmental=environmental) + + if completion: + monkeypatch.setattr(hist.completion, 'insert', raise_error) + else: + monkeypatch.setattr(hist, 'insert', raise_error) + + if environmental: + with caplog.at_level(logging.ERROR): + hist.add_url(QUrl('https://www.example.org/')) + msg = message_mock.getmsg(usertypes.MessageLevel.error) + assert msg.text == "Failed to write history: Error message" + else: + with pytest.raises(sql.SqlError): + hist.add_url(QUrl('https://www.example.org/')) + + @pytest.mark.parametrize('level, url, req_url, expected', [ (logging.DEBUG, 'a.com', 'a.com', [('a.com', 'title', 12345, False)]), (logging.DEBUG, 'a.com', 'b.com', [('a.com', 'title', 12345, False), diff --git a/tests/unit/misc/test_sql.py b/tests/unit/misc/test_sql.py index 953c8c498..fdd5c89c8 100644 --- a/tests/unit/misc/test_sql.py +++ b/tests/unit/misc/test_sql.py @@ -22,10 +22,63 @@ import pytest from qutebrowser.misc import sql +from PyQt5.QtSql import QSqlError + pytestmark = pytest.mark.usefixtures('init_sql') +def test_sqlerror(): + text = "Hello World" + err = sql.SqlError(text, environmental=True) + assert str(err) == text + assert err.text() == text + assert err.environmental + + +class TestSqliteError: + + @pytest.mark.parametrize('error_code, environmental', [ + ('9', True), # SQLITE_LOCKED + ('19', False), # SQLITE_CONSTRAINT + ]) + def test_environmental(self, error_code, environmental): + sql_err = QSqlError("driver text", "db text", QSqlError.UnknownError, + error_code) + err = sql.SqliteError("Message", sql_err) + assert err.environmental == environmental + + def test_logging(self, caplog): + sql_err = QSqlError("driver text", "db text", QSqlError.UnknownError, + '23') + sql.SqliteError("Message", sql_err) + lines = [r.message for r in caplog.records] + expected = ['SQL error:', + 'type: UnknownError', + 'database text: db text', + 'driver text: driver text', + 'error code: 23'] + + 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 + + def test_subclass(self): + with pytest.raises(sql.SqlError): + raise sql.SqliteError("text", QSqlError()) + + def test_text(self): + sql_err = QSqlError("driver text", "db text") + err = sql.SqliteError("Message", sql_err) + assert err.text() == "db text" + + def test_init(): sql.SqlTable('Foo', ['name', 'val', 'lucky']) # should not error if table already exists