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
gh-98893: Improved error messages for sqlite3 UDFs #113762
base: main
Are you sure you want to change the base?
gh-98893: Improved error messages for sqlite3 UDFs #113762
Conversation
erlend-aasland
commented
Jan 6, 2024
•
edited by bedevere-app
bot
edited by bedevere-app
bot
- Issue: sqlite3 - preserve error information from a user-defined function exception #98893
Modules/_sqlite/connection.c
Outdated
if (msg == NULL) { | ||
// An exception happened while we tried to build a more detailed | ||
// error message; write this as an unraisable exception and pass | ||
// the preamble as the sqlite3 error message. | ||
void *data = sqlite3_user_data(context); | ||
callback_context *ctx = (callback_context *)data; | ||
PyErr_WriteUnraisable(ctx->callable); | ||
sqlite3_result_error(context, preamble, -1); | ||
} |
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.
We should be able to test this, for example like this:
diff --git a/Lib/test/test_sqlite3/test_userfunctions.py b/Lib/test/test_sqlite3/test_userfunctions.py
index 67a13ad38b..d8b8c0a40e 100644
--- a/Lib/test/test_sqlite3/test_userfunctions.py
+++ b/Lib/test/test_sqlite3/test_userfunctions.py
@@ -24,6 +24,7 @@
import sys
import unittest
import sqlite3 as sqlite
+import test.support
from unittest.mock import Mock, patch
from test.support import bigmemtest, gc_collect
@@ -35,6 +36,10 @@
class CustomException(Exception):
pass
+class EvilException(Exception):
+ def __str__(self):
+ return -1
+
def func_returntext():
return "foo"
def func_returntextwithnull():
@@ -53,6 +58,8 @@ def func_returnlonglong():
return 1<<31
def func_raiseexception():
raise CustomException("func except")
+def func_raiseevil():
+ raise EvilException("my __str__() does not return a string")
def func_memoryerror():
raise MemoryError
def func_overflowerror():
@@ -162,6 +169,7 @@ def setUp(self):
self.con.create_function("return_noncont_blob", 0,
lambda: memoryview(b"blob")[::2])
self.con.create_function("raiseexception", 0, func_raiseexception)
+ self.con.create_function("raiseevil", 0, func_raiseevil)
self.con.create_function("memoryerror", 0, func_memoryerror)
self.con.create_function("overflowerror", 0, func_overflowerror)
@@ -265,6 +273,18 @@ def test_func_exception(self):
cur.execute("select raiseexception()")
cur.fetchone()
+ @with_tracebacks(EvilException, name="func_raiseevil")
+ def test_func_evilexception(self):
+ cur = self.con.cursor()
+ # Since it is impossible to fetch a string with the exception message
+ # from EvilException, we will only receive the generic preamble
+ # exception message, hence the detailed regex.
+ msg = "^user-defined function raised exception$"
+ with self.assertRaisesRegex(sqlite.OperationalError, msg):
+ with test.support.catch_unraisable_exception() as cm:
+ cur.execute("select raiseevil()")
+ cur.fetchone()
+
@with_tracebacks(MemoryError, name="func_memoryerror")
Unfortunately, our @with_tracebacks
decorator gets in the way. I'll think of a way to solve that (we can also handle it later in a follow-up PR).
@serhiy-storchaka, would you like to take a look at this? I'm not fully satisfied with it yet, but I think it is ready for an initial round of review. I believe the error messages can be tuned to be slightly nicer. Also, I'd like to add coverage for one of the special cases, as mentioned in #113762 (comment). |
} | ||
|
||
static char * | ||
build_error_message(const char *preamble, const char *message) |
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.
It may be simpler to use PyUnicode_Format()
.
Also, it is worth to include also the type name of the original error, not only its message.
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.
Thanks, those are good suggestions.