Harden error handling + code-review quick wins (A+B)#142
Merged
Conversation
(A) Error-handling hardening: - Agent.__del__ no longer calls the raising send() on KeyboardInterrupt (the agent is already inactive there, so send() raised AgentError and masked the interrupt) -- now best-effort, never escapes __del__ (C2). - python_source.on_session_start catches Exception, not BaseException, around loadModule, so a slow/wedged import stays interruptible (H1). - list_all_modules._process_package: drop the redundant `except (ImportError, Exception)`; catch Exception and skip the package instead of aborting discovery (H2). - utils.remove_logging_pycache is now best-effort: guards a missing __pycache__ (was an uncaught crash at startup) and wraps the reload (H3). - Application.exit() runs deinitX11()+config=None in `finally`, and runProject()'s teardown destroy() is guarded, so cleanup can't leak X11 access or mask an in-flight exception (H4 + M). - Exception chaining (`raise X from e`) at the re-raise sites in config.py, process/create.py, session_directory.py. (B) Quick wins: - argument_generator imports fusil.python.tricky_weird directly instead of the package hub, removing the fragile runtime import cycle. - oom_dedup._BT_SKIP now skips the eval-loop teardown / generic-dealloc detector frames (_Py_Dealloc, PyStackRef_XCLOSE, _PyFrame_ClearLocals, _PyFrame_ClearExceptCode, clear_thread_frame), in lockstep with the catalog's gen_known_sites.GENERIC_DETECTOR_FUNCS. - Retired the broken standalone test_doc.py (it ran stale Py2-era doc/*.rst doctests); the surviving green module doctests (fusil.tools, fusil.process.tools) now run under unittest via tests/python/test_doctests.py. Suite 320 OK; ruff check + format clean. Remaining backlog (complexity refactors, the softer M-series sentinel warnings, legacy-coupled dead code) stays in doc/code-review-findings.md. Closes #141 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Works through the still-live items from
doc/code-review-findings.md(several originally-listed items were already fixed).(A) Error-handling hardening
Agent.__del__no longer calls the raisingsend()on Ctrl-C (the agent is already inactive there, sosend()raisedAgentErrorand masked the interrupt); now best-effort and never escapes__del__.python_source.on_session_startcatchesException(notBaseException) aroundloadModule, so a slow/wedged import stays interruptible.list_all_modules._process_packagedrops the redundantexcept (ImportError, Exception); catchesExceptionand skips the package instead of aborting discovery.utils.remove_logging_pycacheis best-effort: guards a missing__pycache__(was an uncaught startup crash) and wraps thereload.Application.exit()runsdeinitX11()/config=Noneinfinally;runProject()'s teardowndestroy()is guarded so it can't leak X11 access or mask an in-flight exception.raise X from e) at the re-raise sites inconfig.py,process/create.py,session_directory.py.(B) Quick wins
argument_generatorimportsfusil.python.tricky_weirddirectly instead of thefusil.pythonpackage hub.oom_dedup._BT_SKIPnow skips the eval-loop teardown / generic-dealloc detector frames, in lockstep with the catalog'sgen_known_sites.GENERIC_DETECTOR_FUNCS.test_doc.py(stale Py2-eradoc/*.rstdoctests); the surviving green module doctests (fusil.tools,fusil.process.tools) now run under unittest viatests/python/test_doctests.py.Test plan
ruff check+ruff format --checkclean overfusil/ tests/ fuzzers/fusil-python-threaded.--helpsmoke confirms the import-cycle change keeps the package init + option parser building cleanly.Remaining backlog (complexity refactors, softer M-series sentinel warnings, legacy-coupled dead code) stays documented in
doc/code-review-findings.md.Closes #141