Merge pull request #3073 from qutebrowser/sql-errors
Handle some sqlite errors gracefully
This commit is contained in:
commit
ba92f32e9f
@ -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()
|
||||
|
@ -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'."""
|
||||
|
@ -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()
|
||||
|
||||
|
@ -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),
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user