diff --git a/cortex_viz/server/graph_build_run.py b/cortex_viz/server/graph_build_run.py index 7fbb757..d8600b8 100644 --- a/cortex_viz/server/graph_build_run.py +++ b/cortex_viz/server/graph_build_run.py @@ -542,13 +542,22 @@ def _on_batch(label: str, nodes_objs, edges_objs) -> None: # early return, or error. Subscribers drain whatever was # emitted, receive ``done``, and close. close() is # idempotent; the buffer survives for late-subscriber - # replay until the next build's reset(). + # replay until the next build's reset(). A failure here must + # not mask the build outcome being unwound, so it is never + # re-raised — but not-masking and not-reporting are + # different things (#135): silently swallowing it is exactly + # why #134's AttributeError on this same call went unnoticed + # for a release. Report it on stderr instead; subscribers + # still never receive ``done`` and are left on an open + # connection, but now there is a line naming why. try: from cortex_viz.server import graph_event_stream as _ev _ev.close() - except Exception: - # close() is idempotent and the buffer survives for late-subscriber replay; - # a failure here must not mask the build outcome being unwound. - pass + except Exception as _close_exc: + print( + f"[cortex] end-of-stream terminator failed: " + f"{type(_close_exc).__name__}: {_close_exc}", + file=sys.stderr, + ) state._graph_build_lock.release() diff --git a/tests/test_graph_build_run_terminator.py b/tests/test_graph_build_run_terminator.py new file mode 100644 index 0000000..c7cd571 --- /dev/null +++ b/tests/test_graph_build_run_terminator.py @@ -0,0 +1,69 @@ +"""#135: the end-of-build SSE terminator must report a failure, never mask it. + +``run_build``'s ``finally`` sends the single ``close()`` end-of-stream call. +#134 made ``close()`` resolve again; this file exercises what happens the +*next* time it raises — the handler must not re-raise (a failure here must +not mask the build outcome being unwound) but must no longer swallow the +failure silently either, since that silence is exactly why #134's +``AttributeError`` on this call survived a release unnoticed. + +``run_build`` is exercised directly (not through a fake), forcing the very +first line of its ``try`` to fail so the test reaches ``finally`` without +depending on a real store or database. + +``graph_event_stream.close`` is patched with ``raising=False``: this file is +independent of #134 (a separate PR against the same base), and whether the +module-level ``close`` forwarder exists yet depends on merge order, not on +this fix. Both scenarios must behave identically here — the finally block +must report a failing terminator regardless of *why* it failed. +""" + +from __future__ import annotations + +from cortex_viz.server import graph_build_run +from cortex_viz.server import graph_cache_state as state +from cortex_viz.server import graph_event_stream as ges + + +def _run_with_forced_early_failure(monkeypatch): + """Force ``run_build``'s try block to fail on its first statement, so + the finally block is reached without a store, a DB, or a real build.""" + + def _boom(): + raise RuntimeError("forced early build failure") + + monkeypatch.setattr(graph_build_run, "_roster_fingerprint", _boom) + state._graph_build_lock.acquire() + graph_build_run.run_build(store=object(), domain_filter=None) + + +def test_terminator_failure_is_reported_on_stderr(monkeypatch, capsys): + monkeypatch.setattr( + ges, + "close", + lambda: (_ for _ in ()).throw(AttributeError("no attribute 'close'")), + raising=False, + ) + + _run_with_forced_early_failure(monkeypatch) + + err = capsys.readouterr().err + assert "end-of-stream terminator failed" in err + assert "AttributeError" in err + assert "no attribute 'close'" in err + # Not re-raised: the build's own error report is still the thing that + # reaches stderr as the build outcome, and the lock is still released. + assert "background build error" in err + assert not state._graph_build_lock.locked() + + +def test_nominal_terminator_stays_quiet(monkeypatch, capsys): + closed = [] + monkeypatch.setattr(ges, "close", lambda: closed.append(True), raising=False) + + _run_with_forced_early_failure(monkeypatch) + + err = capsys.readouterr().err + assert closed == [True] + assert "end-of-stream terminator failed" not in err + assert not state._graph_build_lock.locked()