rnhmjoj/qutebrowser
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Redesign SQL error handling

Browse Source 
Instead of having an environmental attribute on exceptions, we now have two
different exception classes.

Fixes #3341
See #3073
This commit is contained in:
Florian Bruhin 2018-09-01 21:14:13 +02:00
parent 99ae49ccd6
commit 50ae2bf2f9
5 changed files with 122 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
qutebrowser/app.py
View File

@ -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()

7
qutebrowser/browser/history.py
View File

@ -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': []}

142
qutebrowser/misc/sql.py
View File

@ -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):

7
tests/unit/browser/test_history.py
View File

@ -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', [

66
tests/unit/misc/test_sql.py
View File

@ -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()