Private
Public Access
feat(scripts): Phase 12.1+12.2+12.3 - remove Heuristic #19; fix visit_Try; add Heuristic D
Phase 12.1: REMOVE Heuristic #19 (narrow except + log = INTERNAL_COMPLIANT). Per error_handling.md Broad-Except Distinction table and the user's principle (2026-06-17): 'logging is NOT a drain'. A catch+log site is INTERNAL_SILENT_SWALLOW (a violation), not INTERNAL_COMPLIANT. The explicit reclassification runs AFTER drain-point checks so a site with BOTH a log call AND a drain point (e.g., sys.stderr.write + sys.exit) is classified by the drain point (which wins). Phase 12.2: FIX the visit_Try audit bug. The walker did NOT recurse into node.body (the try body itself), so nested Trys were silently dropped from the audit. Verified against src/api_hooks.py: 23 actual try/except nodes but only 5 reported — gap of 18 sites, 12+ silent violations. Fix: added 'for child in node.body: self.visit(child)' to ExceptionVisitor.visit_Try (placed before the handlers loop). Phase 12.3: ADD Heuristic D (5 drain-point patterns) with TDD: - D.1 HTTP error response (BaseHTTPRequestHandler.send_response) - D.2 GUI error display (imgui.open_popup) - D.3 Intentional app termination (sys.exit) - D.4 Telemetry emission (telemetry.emit_*) - D.5 Bounded retry (for attempt in range(N): try; return None) Added 5 new helper methods to ExceptionVisitor: _has_send_response_call, _has_imgui_error_display, _has_sys_exit_call, _has_telemetry_emit_call, _has_bounded_retry. Tests: - test_narrow_except_with_log_only_is_silent_swallow (NEW, PASSES) - test_narrow_except_with_logging_error_is_silent_swallow (NEW, PASSES) - test_visit_try_recurses_into_try_body (NEW, PASSES - nested Try) - test_drain_point_http_error_response_is_compliant (NEW, PASSES) - test_drain_point_gui_error_display_is_compliant (NEW, PASSES) - test_drain_point_app_termination_is_compliant (NEW, PASSES) - test_drain_point_telemetry_emit_is_compliant (NEW, PASSES) - test_drain_point_bounded_retry_is_compliant (NEW, PASSES) Test count: 14 baseline + 8 new = 22 total in test_audit_exception_handling_heuristics.py. All 22 pass (20 PASSED + 2 XFAIL from Phase 11's #22/#23 laundering heuristics).
This commit is contained in:
@@ -395,3 +395,220 @@ def test_result_returning_recovery_in_result_named_function_is_compliant():
|
||||
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
|
||||
f"Result-returning recovery in *_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.1: Heuristic #19 REMOVED - narrow except + log is INTERNAL_SILENT_SWALLOW
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_narrow_except_with_log_only_is_silent_swallow():
|
||||
"""try: ...; except (SpecificError): sys.stderr.write(...) is INTERNAL_SILENT_SWALLOW (a violation).
|
||||
|
||||
Per error_handling.md "The Broad-Except Distinction" table and the user's
|
||||
principle (2026-06-17): "logging is NOT a drain". sys.stderr.write alone
|
||||
loses the error context; the propagation does NOT terminate visibly to
|
||||
the user. The convention requires Result[T] propagation to a true drain
|
||||
point. Heuristic #19 (which classified this as compliant) was REMOVED
|
||||
in Phase 12.1.
|
||||
"""
|
||||
src = (
|
||||
'def log_failure(path, e):\n'
|
||||
' try:\n'
|
||||
' path.write_text("x", encoding="utf-8")\n'
|
||||
' except (OSError, UnicodeEncodeError):\n'
|
||||
' sys.stderr.write(f"write failed: {e}")\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_SILENT_SWALLOW", (
|
||||
f"narrow except + log only should be INTERNAL_SILENT_SWALLOW (logging is NOT a drain), got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
|
||||
def test_narrow_except_with_logging_error_is_silent_swallow():
|
||||
"""try: ...; except (SpecificError): logging.error(...) is INTERNAL_SILENT_SWALLOW (a violation).
|
||||
|
||||
Same principle as test_narrow_except_with_log_only_is_silent_swallow
|
||||
but with the logging module. Logging alone loses the error context.
|
||||
"""
|
||||
src = (
|
||||
'def log_failure_via_logging(path):\n'
|
||||
' try:\n'
|
||||
' path.write_text("x", encoding="utf-8")\n'
|
||||
' except (OSError, UnicodeEncodeError) as e:\n'
|
||||
' logging.error(f"write failed: {e}")\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_SILENT_SWALLOW", (
|
||||
f"narrow except + logging.error should be INTERNAL_SILENT_SWALLOW, got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.2: visit_Try recursion fix - nested Trys in try body are visited
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_visit_try_recurses_into_try_body():
|
||||
"""A nested try inside the try body should be visited and its handlers recorded.
|
||||
|
||||
The audit's visit_Try had a bug where it did NOT recurse into node.body.
|
||||
This test constructs a source with an outer try containing an inner try,
|
||||
and asserts BOTH outer and inner handlers appear in the findings.
|
||||
"""
|
||||
src = (
|
||||
'def outer():\n'
|
||||
' try:\n'
|
||||
' try:\n'
|
||||
' do_inner()\n'
|
||||
' except ValueError:\n'
|
||||
' handle_inner()\n'
|
||||
' do_outer_thing()\n'
|
||||
' except (OSError, IOError):\n'
|
||||
' handle_outer()\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 2, (
|
||||
f"visit_Try should recurse into try body; expected 2 EXCEPT findings, got {len(excepts)}: {excepts}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.3: Heuristic D.1 - HTTP error response drain point
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_drain_point_http_error_response_is_compliant():
|
||||
"""try: ...; except (SpecificError): self.send_response(500, ...) is INTERNAL_COMPLIANT (drain point D.1).
|
||||
|
||||
Per error_handling.md Drain Points section, Pattern 1: HTTP error
|
||||
response in a BaseHTTPRequestHandler subclass IS a drain point. The
|
||||
HTTP status code IS the visible user feedback; the propagation
|
||||
terminates at the HTTP response. Heuristic D.1 recognizes this pattern.
|
||||
"""
|
||||
src = (
|
||||
'class Handler(BaseHTTPRequestHandler):\n'
|
||||
' def do_GET(self):\n'
|
||||
' try:\n'
|
||||
' self._read_body()\n'
|
||||
' except (OSError, ValueError) as e:\n'
|
||||
' self.send_response(500)\n'
|
||||
' self.send_header("Content-Type", "application/json")\n'
|
||||
' self.wfile.write(b\'{"error": "internal"}\')\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
|
||||
f"HTTP error response should be INTERNAL_COMPLIANT (drain point D.1), got {excepts[0]['category']}: {excepts[0].get('note', '')}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.3: Heuristic D.2 - GUI error display drain point
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_drain_point_gui_error_display_is_compliant():
|
||||
"""try: ...; except (SpecificError): imgui.open_popup(...) is INTERNAL_COMPLIANT (drain point D.2).
|
||||
|
||||
Per error_handling.md Drain Points section, Pattern 2: GUI error
|
||||
display via imgui.open_popup IS a drain point. The user sees the
|
||||
error modal.
|
||||
"""
|
||||
src = (
|
||||
'def show_load_error():\n'
|
||||
' try:\n'
|
||||
' do_load()\n'
|
||||
' except (OSError, ValueError):\n'
|
||||
' imgui.open_popup("Load Error")\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
|
||||
f"GUI error display should be INTERNAL_COMPLIANT (drain point D.2), got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.3: Heuristic D.3 - Intentional app termination drain point
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_drain_point_app_termination_is_compliant():
|
||||
"""try: ...; except (SpecificError): sys.exit(1) is INTERNAL_COMPLIANT (drain point D.3).
|
||||
|
||||
Per error_handling.md Drain Points section, Pattern 3: intentional
|
||||
app termination via sys.exit IS a drain point. The process exit IS
|
||||
the termination of the propagation.
|
||||
"""
|
||||
src = (
|
||||
'def critical_init():\n'
|
||||
' try:\n'
|
||||
' load_config()\n'
|
||||
' except (OSError, ValueError):\n'
|
||||
' sys.stderr.write("FATAL: config missing")\n'
|
||||
' sys.exit(1)\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
|
||||
f"app termination should be INTERNAL_COMPLIANT (drain point D.3), got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.3: Heuristic D.4 - Telemetry emission drain point
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_drain_point_telemetry_emit_is_compliant():
|
||||
"""try: ...; except (SpecificError): telemetry.emit_error(...) is INTERNAL_COMPLIANT (drain point D.4).
|
||||
|
||||
Per error_handling.md Drain Points section, Pattern 4: telemetry
|
||||
emission IS a drain point. The error reaches the monitoring system.
|
||||
"""
|
||||
src = (
|
||||
'def report_failure():\n'
|
||||
' try:\n'
|
||||
' do_thing()\n'
|
||||
' except (OSError, ValueError):\n'
|
||||
' telemetry.emit_error(operation="do_thing", kind="INTERNAL", message="failed")\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
|
||||
f"telemetry emit should be INTERNAL_COMPLIANT (drain point D.4), got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 12.3: Heuristic D.5 - Bounded retry drain point
|
||||
# ---------------------------------------------------------------------------
|
||||
def test_drain_point_bounded_retry_is_compliant():
|
||||
"""try: ...; except (SpecificError): for attempt in range(3): ...; return None is INTERNAL_COMPLIANT (drain point D.5).
|
||||
|
||||
Per error_handling.md Drain Points section, Pattern 5: bounded retry
|
||||
followed by return None IS a drain point. The retry is bounded (no
|
||||
infinite loop); the final None propagates to a visible error UI.
|
||||
"""
|
||||
src = (
|
||||
'def load_with_retry():\n'
|
||||
' for attempt in range(3):\n'
|
||||
' try:\n'
|
||||
' do_load()\n'
|
||||
' return "ok"\n'
|
||||
' except (OSError, ValueError):\n'
|
||||
' time.sleep(1)\n'
|
||||
' return None\n'
|
||||
)
|
||||
data = _run_audit_on_fixture(src)
|
||||
findings = _classifications_for_file(data, "audit_heuristic_fixture.py")
|
||||
excepts = [f for f in findings if f["kind"] == "EXCEPT"]
|
||||
assert len(excepts) == 1
|
||||
assert excepts[0]["category"] == "INTERNAL_COMPLIANT", (
|
||||
f"bounded retry should be INTERNAL_COMPLIANT (drain point D.5), got {excepts[0]['category']}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user