From ed8ec5d5c8815790f3918cab241a1cbce5692050 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 9 Jul 2026 13:23:23 +0300 Subject: [PATCH] fix(parsers/python): guard extraction so one bad file can't abort the whole repo parse FunctionExtractor.process_file guarded only ast.parse (except SyntaxError); the extraction body and both caller loops (extract_from_scan, extract_all) were unguarded, so a single pathological file raising a non-SyntaxError -- e.g. a RecursionError from a deeply-nested source, or any error in the tree walk -- propagated out of the file loop and aborted the entire parse, zeroing ALL units. - Add a per-file guard _process_file_guarded (try/except Exception -> log a WARNING to stderr + files_with_errors += 1) at both loops, mirroring the Zig/Go per-file guards (parsers/zig/function_extractor.py:57-77), the only currently-immune parsers. - Move files_processed += 1 to the end of process_file so a file that crashes mid-extraction is counted once as an error and never as processed (no double-count). - except Exception (not BaseException) so KeyboardInterrupt/SystemExit still propagate; the failure is logged, never silently swallowed. Verified against origin/master (98f5504): byte-identical unit-id set + stats on click/flask/httpie/rich via both extract_from_scan and extract_all (no change on valid code); recovers every real unit when one file raises at parse OR post-parse; python parser test suite unchanged vs base. Regression test (fails on master, passes here): $ python -m pytest tests/parsers/python/test_function_extractor_robustness.py -q 4 passed Does NOT change valid-file extraction output, the func_id scheme, or any other parser. The same latent pattern in the JS/C/Ruby/PHP extractors is a tracked follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../parsers/python/function_extractor.py | 23 +++++- .../test_function_extractor_robustness.py | 73 +++++++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 libs/openant-core/tests/parsers/python/test_function_extractor_robustness.py diff --git a/libs/openant-core/parsers/python/function_extractor.py b/libs/openant-core/parsers/python/function_extractor.py index 93f7ebc3..e639332d 100644 --- a/libs/openant-core/parsers/python/function_extractor.py +++ b/libs/openant-core/parsers/python/function_extractor.py @@ -759,7 +759,6 @@ def process_file(self, file_path: Path) -> None: self.stats['files_with_errors'] += 1 return - self.stats['files_processed'] += 1 relative_path = file_path.relative_to(self.repo_path).as_posix() # Extract imports @@ -790,11 +789,27 @@ def process_file(self, file_path: Path) -> None: self.stats['module_level_units'] += 1 self.stats['by_type']['module_level'] = self.stats['by_type'].get('module_level', 0) + 1 + # Count as processed only after extraction fully succeeds, so a file that crashes + # mid-extraction (caught by _process_file_guarded) is counted once as an error and + # never as processed. + self.stats['files_processed'] += 1 + + def _process_file_guarded(self, file_path: Path) -> None: + """Process one file, isolating any failure so a single pathological file cannot + abort the whole repo's extraction (mirrors the Zig/Go per-file loop guard). + ast.parse SyntaxErrors are handled inside process_file; this backstops the rest + (RecursionError/MemoryError from deep nesting, ValueError, extractor bugs).""" + try: + self.process_file(file_path) + except Exception as e: + print(f"Warning: failed to process {file_path}: {type(e).__name__}: {e}", file=sys.stderr) + self.stats['files_with_errors'] += 1 + def extract_from_scan(self, scan_result: Dict) -> Dict: """Extract functions from files listed in a scan result.""" for file_info in scan_result.get('files', []): file_path = self.repo_path / file_info['path'] - self.process_file(file_path) + self._process_file_guarded(file_path) return self.export() @@ -804,7 +819,7 @@ def extract_all(self, files: Optional[List[str]] = None) -> Dict: for file_rel_path in files: file_path = self.repo_path / file_rel_path if file_path.exists(): - self.process_file(file_path) + self._process_file_guarded(file_path) else: # Scan all .py files for file_path in self.repo_path.rglob('*.py'): @@ -815,7 +830,7 @@ def extract_all(self, files: Optional[List[str]] = None) -> Dict: excluded = {'__pycache__', '.git', 'venv', '.venv', 'node_modules'} if excluded & set(file_path.relative_to(self.repo_path).parts): continue - self.process_file(file_path) + self._process_file_guarded(file_path) return self.export() diff --git a/libs/openant-core/tests/parsers/python/test_function_extractor_robustness.py b/libs/openant-core/tests/parsers/python/test_function_extractor_robustness.py new file mode 100644 index 00000000..38274d98 --- /dev/null +++ b/libs/openant-core/tests/parsers/python/test_function_extractor_robustness.py @@ -0,0 +1,73 @@ +"""Regression locks: one pathological Python file must not abort the whole repo's extraction. + +Before the fix, `FunctionExtractor.process_file` guarded only `ast.parse` (`except SyntaxError`), +while the extraction body and both caller loops (`extract_from_scan`, `extract_all`) were +unguarded. A single file raising a non-SyntaxError (e.g. `RecursionError` from a deeply-nested +source, or any error in the tree walk) propagated out of the file loop and aborted the entire +parse, losing ALL units collected so far. The fix adds a per-file guard at the loop +(`_process_file_guarded`), mirroring the Zig/Go parsers. + +These tests drive the real extractor entry points (extract_from_scan = the production path used +by parse_repository, and extract_all), not hand-constructed dicts. +""" +import tempfile +from pathlib import Path + +from parsers.python.function_extractor import FunctionExtractor + + +def _repo(files: dict) -> str: + d = Path(tempfile.mkdtemp()) + for name, src in files.items(): + (d / name).write_text(src) + return str(d) + + +# a deeply-nested attribute chain overflows CPython's stack inside ast.parse -> RecursionError +_STACK_BLOWER = "x = a" + ".b" * 60000 + "\n" + + +def test_pathological_file_does_not_abort_extract_from_scan(): + repo = _repo({"good.py": "def alpha(x):\n return x + 1\n", "bad.py": _STACK_BLOWER}) + ex = FunctionExtractor(repo) + res = ex.extract_from_scan({"files": [{"path": "good.py"}, {"path": "bad.py"}]}) + assert "good.py:alpha" in res["functions"], "good file's units lost — a bad file aborted the parse" + assert res["statistics"]["files_with_errors"] >= 1 + + +def test_pathological_file_does_not_abort_extract_all(): + repo = _repo({"good.py": "def beta(a, b):\n return a + b\n", "bad.py": _STACK_BLOWER}) + res = FunctionExtractor(repo).extract_all(["good.py", "bad.py"]) + assert "good.py:beta" in res["functions"] + + +def test_post_parse_extraction_error_is_isolated(): + """A crash AFTER ast.parse (in the tree walk) must also be isolated — the guard is at the + loop, not just around ast.parse, so it covers the extraction body too.""" + repo = _repo({ + "good.py": "def gamma():\n return 3\n", + "boom.py": "def trigger_boom():\n return 1\n", + }) + ex = FunctionExtractor(repo) + orig = ex.process_function + + def boom(node, *a, **k): + if getattr(node, "name", "") == "trigger_boom": + raise RecursionError("simulated deep extraction recursion") + return orig(node, *a, **k) + + ex.process_function = boom + res = ex.extract_from_scan({"files": [{"path": "good.py"}, {"path": "boom.py"}]}) + assert "good.py:gamma" in res["functions"], "post-parse crash in one file aborted the batch" + assert res["statistics"]["files_with_errors"] >= 1 + + +def test_stats_not_double_counted(): + repo = _repo({ + "g1.py": "def a():\n return 1\n", + "g2.py": "def b():\n return 2\n", + "bad.py": _STACK_BLOWER, + }) + st = FunctionExtractor(repo).extract_all(["g1.py", "g2.py", "bad.py"])["statistics"] + assert st["files_processed"] == 2, "a crashed file must not be counted as processed" + assert st["files_with_errors"] == 1