-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: enable tracebacks for "user"/custom sqlite functions #5383
base: master
Are you sure you want to change the base?
Conversation
A bit niche but I tried setting my bareasc prefix to an empty string, and was getting an obtuse error. This should help make clearer what is happening when queries fail. The exception is not properly raised up the stack in the first place because it happens across 2 FFI boundaries: the DB query (Python -> SQLite), and the custom DB function (SQLite -> Python). Thus Python cannot forwarded it back to itself through SQLite, and it's treated as an "unraisable" exception. We could override `sys.unraisablehook` to not print anything for the original exception, and store it in a global for the outer Python interpreter to fetch and raise properly, but that's pretty hacky, limited to a single DB instance and query at once, and risks swallowing other "unraisable" exceptions. Instead we just tell the user to look above for what Python prints. Sample output: ``` Exception ignored in: <function unidecode_expect_ascii at 0x7f7fa20bb060> Traceback (most recent call last): File "site-packages/unidecode/__init__.py", line 60, in unidecode_expect_ascii bytestring = string.encode('ASCII') ^^^^^^^^^^^^^ AttributeError: 'bytes' object has no attribute 'encode' Traceback (most recent call last): File "site-packages/beets/dbcore/db.py", line 988, in query cursor = self.db._connection().execute(statement, subvals) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sqlite3.OperationalError: user-defined function raised exception During handling of the above exception, another exception occurred: Traceback (most recent call last): File "site-packages/beets/__main__.py", line 9, in <module> sys.exit(main()) ^^^^^^ File "site-packages/beets/ui/__init__.py", line 1865, in main _raw_main(args) File "site-packages/beets/ui/__init__.py", line 1852, in _raw_main subcommand.func(lib, suboptions, subargs) File "site-packages/beets/ui/commands.py", line 1599, in list_func list_items(lib, decargs(args), opts.album) File "site-packages/beets/ui/commands.py", line 1594, in list_items for item in lib.items(query): ^^^^^^^^^^^^^^^^ File "site-packages/beets/library.py", line 1695, in items return self._fetch(Item, query, sort or self.get_default_item_sort()) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/beets/library.py", line 1673, in _fetch return super()._fetch(model_cls, query, sort) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/beets/dbcore/db.py", line 1301, in _fetch rows = tx.query(sql, subvals) ^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/beets/dbcore/db.py", line 991, in query raise DBCustomFunctionError() beets.dbcore.db.DBCustomFunctionError: beets defined SQLite function failed; see the other errors above for details ```
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
@@ -2,8 +2,7 @@ name: Test | |||
on: | |||
pull_request: | |||
push: | |||
branches: | |||
- master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these two changes in github workflows intended?
@@ -1028,6 +1045,10 @@ def __init__(self, path, timeout: float = 5.0): | |||
"sqlite3 must be compiled with multi-threading support" | |||
) | |||
|
|||
# Print tracebacks for exceptions in user defined functions | |||
# See also `self.add_functions` and `DBCustomFunctionError`. | |||
sqlite3.enable_callback_tracebacks(True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you reckon we should only do this if the logging level is DEBUG
?
deterministic = {} | ||
if sys.version_info >= (3, 8) and sqlite_version_info >= (3, 8, 3): | ||
# Let sqlite make extra optimizations | ||
deterministic["deterministic"] = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using partial
here, it may make it just slightly cleaner
create_function = conn.create_function
...
create_function = partial(create_function, deterministic=True)
create_function("regexp", 2, regexp)
...
Description
A bit niche but I tried setting my bareasc prefix to an empty string,
and was getting an obtuse error. This should help make clearer what is
happening when queries fail.
The exception is not properly raised up the stack in the first place
because it happens across 2 FFI boundaries: the DB query
(Python -> SQLite), and the custom DB function (SQLite -> Python).
Thus Python cannot forwarded it back to itself through SQLite, and it's
treated as an "unraisable" exception.
We could override
sys.unraisablehook
to not print anything for theoriginal exception, and store it in a global for the outer Python
interpreter to fetch and raise properly, but that's pretty hacky,
limited to a single DB instance and query at once, and risks swallowing
other "unraisable" exceptions.
Instead we just tell the user to look above for what Python prints.
Sample output:
To Do
Documentation. (If you've added a new command-line flag, for example, find the appropriate page underdocs/
to describe it.)Changelog. (Add an entry todocs/changelog.rst
to the bottom of one of the lists near the top of the document.)