By default, sqlite3 will suppress exceptions that happen in during user-defined callbacks (UDF’s, progress callbacks, etc.) and instead raise sqlite3.OperationalError exceptions with generic error messages like “user-defined aggregate’s ‘finalize’ method raised error”. In order to get a better idea about what happened, you’ll have to enable callback tracebacks:
>>> import sqlite3
c>>> cx = sqlite3.connect(":memory:")
>>> cx.create_function("test", 0, lambda: 5/0)
>>> cx.execute("select test()")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
cx.execute("select test()")
sqlite3.OperationalError: user-defined function raised exception
>>> sqlite3.enable_callback_tracebacks(True)
>>> cx.execute("select test()")
Exception ignored in: <function <lambda> at 0x100eed4e0>
Traceback (most recent call last):
File "<stdin>-2", line 1, in <lambda>
ZeroDivisionError: division by zero
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
cx.execute("select test()")
sqlite3.OperationalError: user-defined function raised exception
>>>
I’d like to change this behaviour so we raise unraisable exceptions by default; IOW, make sqlite3.enable_callback_tracebacks a no-op.
As someone who’s very much a non-expert in SQLlite, is there a reason why this wasn’t enabled by deafult previously? And might it be better to flip the default for enable_callback_tracebacks but still allow disabling them (since that function allows toggling either), particularly since I imagine it might have backward-compatibility implications if it changes the exception being emitted (and thus handled)?
Unraisable exceptions are not “emitted” like normal exceptions. The default unraisable hook only prints to stderr, IIRC. Allowing sqlite3.enable_callback_tracebacks(False) sounds fair.
So, the real question is; does this warrant a deprecation warning because of the change in behaviour? IMO, it should be ok to alter the current default without a deprecation warning.
Just to make sure I understand what you’re suggesting:
The current behavior is to raise an OperationalError with a generic
message and you want to change this to always show a full traceback with
the original error ?
This sounds like a good idea, since masking errors often makes debugging
harder.
Yep. OperationalError will still be the exception the user sees. The underlying exception will be raised as an unraisable exception, so you’d need to install a custom sys.unraisablehook in order to intercept it. Alternatively, we could piggyback it onto the OperationalError, but that is not as straight-forwards as it may seem: there may be multiple unraisable exceptions before OperationalError is delivered.
Why not store the exception somewhere and re-use it as cause or context of the OperationalError that’s raised later? That would be much better than dumping an error message.
AFAIK, the unraisable errors reserved for the special case where there
is no other way to report these errors upstream, e.g. during shutdown.
I understand that in the ideal case, it would be good to catch
individual UDF errors, but if that’s not possible, adding all the errors
to a single OperationalError raised after the call triggering the UDFs
is the next best thing.
Since you don’t change the exception type, this could also be done
without deprecation period, since you’re not really changing any
documented behavior.
FWIW I’m still a little confused (having never used this API) about what prompted the original design. Is the problem that there’s no “error return” from a callback, because sqlite doesn’t support that, so you squirrel the error away in a global variable and check that variable later when you’re at a point where you can return an error?
The problem is that OperationalError is the part of the specification, so sqlite3 cannot re-raise an arbitrary exception, but must raise an OperationalError. Even if the user presses Ctrl-C, it only interrupts the callback code, the caller of the sqlite3 API will get a general OperationalError.
But the original exception can be attached to OperationalError.
Cool, this could go through __cause__ or __context__ (I can never remember which is which :-). If there are multiple callback errors they could be combined using an ExceptionGroup.
Yeah, my first thought was ExceptionGroups too, but that wouldn’t be backward compatible or follow the DB-API spec, but this seems like a case for raise OperationalError from user_callback_error (which sets __cause__; its __context__ that’s set automatically; the way I remember it is context is implicit, while cause is explicit)—assuming its possible to actually get the original exception object.
We propose to add two new builtin exception types: BaseExceptionGroup(BaseException) and ExceptionGroup(BaseExceptionGroup, Exception). They are assignable to Exception.__cause__ and Exception.__context__, and they can be raised and handled as any exception with raise ExceptionGroup(...) and try: ... except ExceptionGroup: ... or raise BaseExceptionGroup(...) and try: ... except BaseExceptionGroup: ....
The challenge I expect this to create for existing code is that code that is catching OperationalError to deal with things today will now have unplanned output on stderr that the code was already handling on its own without emitting an error.
That has the potential to mislead users of a functioning program run on an updated version unless the code is updated to silence them via an explicit sqlite3.enable_callback_tracebacks(False) call.
But it isn’t really a logic change. No new exceptions are surfaced or return values change, only some additional stderr/unraisablehook output. Calling the API to squelch that in their code today would be perfectly fine.
If this were called out in What’s New it seems good enough. People getting code running on 3.13 who use sqlite3 would see that and understand if they need to do something. I expect people who’ve needed programatic access to errors from callbacks in sqlite have already surrounded the internals of their callbacks with a try: except: to catch and store it themselves today.
Regardless, I agree that attaching an ExceptionGroup to the OperationalError would be ideal. Nobody ever wants to override sys.unraisablehook to get at details - that is always a hack.
>>> try:
... raise ValueError('original')
... except ValueError as exc:
... try:
... 1/0
... except ZeroDivisionError as exc2:
... e = exc2
... raise
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
ValueError: original
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 5, in <module>
ZeroDivisionError: division by zero
>>>
>>> e
ZeroDivisionError('division by zero')
>>> e.__cause__
>>> e.__context__
ValueError('original')
Another option would be an OperationalExceptionGroup, derived from both ExceptionGroup and OperationalError. That should be both relatively backwards compatible, and usable with except* directly. But it’s not a well-traveled path to take.
This might be obvious to everyone here, but:
If an API can raise ExceptionGroup for something, then it should wrap even a single error in ExceptionGroup. That way, users only need to write except*.
But does that also apply when the exception group appears in __context__ or __cause__? I don’t think so, since you can’t catch those using except, with or without *.