-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Description
One of the area in which pyscript-next is not yet on feature parity with pyscript-classic is error reporting. The goal of this issue is to summarize what are the features that we want in order to improve that.
I noticed that when we talk about "pyscript devs" or "users" it's easy to get confused because there is some ambiguity, so let's start by giving some definitions:
pyscript devs
: the people who develops pyscript: i.e. us :)pyscript users
: the people who develops apps with pyscript.final users
: the people using the apps developed bypyscript users
.
About errors, we have:
internal errors
: errors caused by a bug in pyscript. Ideally, these should not exist (but we are not in an ideal world, so we all know they will :))user errors
: errors caused bypyscript users
not doing things properly: e.g., Python-level exceptions, wrong URLs, wrong config files, etc.app errors
: errors caused by the final users when using the app, such as clicking on the wrong button, putting letters when a number is expected, etc. This kind of error is explicitly excluded from this conversation.
The goal of "improved error reporting" is to make life easier to pyscript users
. It is not about making things nicer for final users
(apart very indirectly because if pyscript users
can work better, they can make better apps).
Rationale and current status in pyscript-classic
Many of the design decisions which I took in pyscript-classic w.r.t. error reporting are based on the following assumptions and goals:
- assume that the Chrome Dev Tools don't exist. Many
pyscript users
are not experienced developers and they might have never heard of dev tools. Moreover, on mobile devices (and maybe tablets) dev tools might not exist at all - assume that
pyscript users
do not read messages printed to the console. This derives directly from the point above. user errors
should be displayed in the DOMuser warnings
should be displayed in the DOM. This is annoying forfinal users
, but it's the job ofpyscript users
to silence them.user errors
and warnings should ideally include a link to the relevant docs, if applicable- errors should never pass silently: for example, if there is a key in the config file which is unknown, we should assume that it's a typo and report it accordingly (either via a warning or an error)
- unless explicitly silenced:
pyscript users
should have a way to customize and/or turn off the default behavior. For example, "display errors in the DOM" should be the default, but they should be able to change this.
In pyscript-classic, the behavior above is implemented in the following way:
-
throwing subclasses of
UserError
: if we detect something wrong, we can justthrow new UserError()
and be sure that it will be displayed to the user in the most appropriate way. Example:
pyscript/pyscriptjs/src/pyconfig.ts
Lines 151 to 154 in 55cf6b9
throw new UserError( ErrorCode.BAD_CONFIG, `The config supplied: ${configText} is an invalid TOML and cannot be parsed`, ); -
UserError
s are handled here:
pyscript/pyscriptjs/src/main.ts
Lines 98 to 109 in 55cf6b9
// Error handling logic: if during the execution we encounter an error // which is ultimate responsibility of the user (e.g.: syntax error in the // config, file not found in fetch, etc.), we can throw UserError(). It is // responsibility of main() to catch it and show it to the user in a // proper way (e.g. by using a banner at the top of the page). async main() { try { await this._realMain(); } catch (error) { await this._handleUserErrorMaybe(error); } } -
Whenever we run Python code, we try/catch errors and display it accordingly:
pyscript/pyscriptjs/src/pyexec.ts
Lines 28 to 38 in 55cf6b9
try { return await interpreter.run(pysrc, outElem.id); } catch (e) { const err = e as Error; // XXX: currently we display exceptions in the same position as // the output. But we probably need a better way to do that, // e.g. allowing plugins to intercept exceptions and display them // in a configurable way. displayPyException(err, outElem); return { result: undefined }; }
Note that even pyscript-classic doesn't always obey to these rules, but it should be considered a bug.
For example, there are a few cases in pyscript-classic in which we incorrectly use console.warn
to emit a warning (which is not seen by anyone in 99% of the cases):
pyscript/pyscriptjs/src/plugins/python/py_tutor.py
Lines 6 to 9 in 55cf6b9
js.console.warn( | |
"WARNING: This plugin is still in a very experimental phase and will likely change" | |
" and potentially break in the future releases. Use it with caution." | |
) |
pyscript/pyscriptjs/src/python/pyscript/_html.py
Lines 22 to 31 in 55cf6b9
def write(element_id, value, append=False, exec_id=0): | |
"""Writes value to the element with id "element_id""" | |
Element(element_id).write(value=value, append=append) | |
js.console.warn( | |
dedent( | |
"""PyScript Deprecation Warning: PyScript.write is | |
marked as deprecated and will be removed sometime soon. Please, use | |
Element(<id>).write instead.""" | |
) | |
) |
All of the behaviors described above are fully tested by integration tests. For example:
pyscript/pyscriptjs/tests/integration/test_01_basic.py
Lines 69 to 94 in 55cf6b9
def test_python_exception(self): | |
self.pyscript_run( | |
""" | |
<py-script> | |
print('hello pyscript') | |
raise Exception('this is an error') | |
</py-script> | |
""" | |
) | |
assert "hello pyscript" in self.console.log.lines | |
self.check_py_errors("Exception: this is an error") | |
# | |
# check that we sent the traceback to the console | |
tb_lines = self.console.error.lines[-1].splitlines() | |
assert tb_lines[0] == "[pyexec] Python exception:" | |
assert tb_lines[1] == "Traceback (most recent call last):" | |
assert tb_lines[-1] == "Exception: this is an error" | |
# | |
# check that we show the traceback in the page. Note that here we | |
# display the "raw" python traceback, without the "[pyexec] Python | |
# exception:" line (which is useful in the console, but not for the | |
# user) | |
pre = self.page.locator("py-script > pre") | |
tb_lines = pre.inner_text().splitlines() | |
assert tb_lines[0] == "Traceback (most recent call last):" | |
assert tb_lines[-1] == "Exception: this is an error" |
For internal errors
, we don't do anything special, AFAIK: i.e., they are normal JS exceptions which are just displayed in the console. (Not saying that this is the right thing to do, just explaining what is the current behavior.
The exact styling for errors and warnings is not set in stone, of course. Ideally, it should be customizable and configurable in various ways (e.g., by having a "notification icon" which displays errors when clicked), but we never managed to implement that.
pyscript-next
I believe that we should follow the same rationale and goals for pyscript-next, and we should implement all the feature described above.
This includes e.g. errors which happens inside workers, which should somehow passed to the main thread and displayed in the DOM.
What belongs to polyscript and what to pyscript is unclear to me. I am happy with whatever decision as long as the pyscript users
see the behavior above.
We could also reconsider what to do for internal errors
: it might be a good idea to show them very explicitly, so that it's easier for people to open a bug report to pyscript (instead of just saying "my app doesn't work" or "pyscript doesn't work").