Change Pyodide code to ensure all fatals are reported.#6334
Change Pyodide code to ensure all fatals are reported.#6334
Conversation
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This change patches Pyodide's fatal_error to invoke API.on_fatal(e) in the recursive-call path, ensuring all fatal errors are reported rather than only the first one.
Issues found (ranked by severity):
- [MEDIUM] Missing try/catch around
on_fatalin the recursive path. In the original non-recursive path,API.on_fatal(e)is called inside atry { ... } catchblock. But in this patch, the call is unguarded. Ifon_fatalitself throws, thereturnstatement won't execute, andfatal_errorwill continue into the rest of its body, which could cause an infinite recursion or other unexpected behavior.
These comments were generated by an AI code review assistant and may contain errors. Please verify all suggestions before applying.
|
Review posted successfully on PR #6334. Summary: The PR patches Pyodide's |
Co-authored-by: dom96 <dom96@users.noreply.github.com>
72ed3b2 to
10d9ae6
Compare
10d9ae6 to
34a6e49
Compare
|
Running |
6159738 to
2d87e14
Compare
The code for this is here: https://github.com/pyodide/pyodide/blob/0.28.2/src/core/error_handling.ts#L92. The on_fatal method is only called when the fatal happens for the first time. This change should ensure it gets called even if the fatal occurred already.