Keep a failed node's traceback on the run
The worker already sent it and the log panel already got it; the failure outcome kept the one-line error and the node's stdout and dropped the rest, so reading a failure back meant reproducing it under `run --local`. It rides in the node's logs now — no schema change, and the API row, the run detail page and `RunHandle.failures` carry it as they are. `_record_node` keeps the tail of the log cap rather than the head, so a chatty node cannot push the traceback past it, and `fluksio run` prints what each node said when a run does not end ok. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TXQv6KNyyvY7Z1etYTUUAd
This commit is contained in:
@@ -1114,7 +1114,10 @@ class Pipeline:
|
|||||||
ok=False,
|
ok=False,
|
||||||
duration_ms=round((time.perf_counter() - started) * 1000, 2),
|
duration_ms=round((time.perf_counter() - started) * 1000, 2),
|
||||||
error=error,
|
error=error,
|
||||||
logs=collected.text,
|
# The run record is the only place a failure is read
|
||||||
|
# back from after the fact, so it carries the traceback
|
||||||
|
# the log panel got rather than the one line alone.
|
||||||
|
logs=collected.text + logs.node_traceback(),
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
return None
|
return None
|
||||||
|
|||||||
@@ -1248,7 +1248,9 @@ class RunService:
|
|||||||
started_at=datetime.now(UTC),
|
started_at=datetime.now(UTC),
|
||||||
duration_ms=outcome.duration_ms,
|
duration_ms=outcome.duration_ms,
|
||||||
error=outcome.error[:ERROR_CAP],
|
error=outcome.error[:ERROR_CAP],
|
||||||
logs=outcome.logs[:LOG_CAP],
|
# The tail, not the head: a failure appends its traceback, and a
|
||||||
|
# chatty node would otherwise push it past the cap.
|
||||||
|
logs=outcome.logs[-LOG_CAP:],
|
||||||
cached_from=outcome.cached_from,
|
cached_from=outcome.cached_from,
|
||||||
# Together or not at all: a row carrying a key must be one a
|
# Together or not at all: a row carrying a key must be one a
|
||||||
# lookup can actually restore from.
|
# lookup can actually restore from.
|
||||||
|
|||||||
@@ -574,6 +574,20 @@ def _cancel(client: Client, handle: RunHandle) -> int:
|
|||||||
return 130
|
return 130
|
||||||
|
|
||||||
|
|
||||||
|
def _report_failures(handle: RunHandle) -> None:
|
||||||
|
"""What each failed node said, traceback included.
|
||||||
|
|
||||||
|
The alternative is reproducing the run under ``--local`` to see it, which
|
||||||
|
is the whole reason the traceback is stored.
|
||||||
|
"""
|
||||||
|
for failed in handle.failures:
|
||||||
|
_say(
|
||||||
|
f" {failed.get('node', '')} {_status('error')} {failed.get('error', '')}"
|
||||||
|
)
|
||||||
|
for line in str(failed.get("logs") or "").splitlines():
|
||||||
|
_say(f" {line}")
|
||||||
|
|
||||||
|
|
||||||
def _cached_note(client: Client, handle: RunHandle) -> str:
|
def _cached_note(client: Client, handle: RunHandle) -> str:
|
||||||
"""How much of the run earlier ones had already answered."""
|
"""How much of the run earlier ones had already answered."""
|
||||||
try:
|
try:
|
||||||
@@ -629,7 +643,10 @@ def cmd_run(args: argparse.Namespace, rest: list[str]) -> int:
|
|||||||
f"{handle.id} {_status(handle.status)} "
|
f"{handle.id} {_status(handle.status)} "
|
||||||
f"{json.dumps(handle.result)}{_cached_note(client, handle)}"
|
f"{json.dumps(handle.result)}{_cached_note(client, handle)}"
|
||||||
)
|
)
|
||||||
return 0 if handle.status == "ok" else 1
|
if handle.status == "ok":
|
||||||
|
return 0
|
||||||
|
_report_failures(handle)
|
||||||
|
return 1
|
||||||
except (SyncError, ApiError) as exc:
|
except (SyncError, ApiError) as exc:
|
||||||
return _fail(str(exc))
|
return _fail(str(exc))
|
||||||
except httpx.HTTPError as exc:
|
except httpx.HTTPError as exc:
|
||||||
|
|||||||
@@ -120,6 +120,10 @@ def test_observer_reports_a_failing_node_with_its_error():
|
|||||||
assert len(seen) == 1
|
assert len(seen) == 1
|
||||||
assert not seen[0].ok
|
assert not seen[0].ok
|
||||||
assert "no convergence" in seen[0].error
|
assert "no convergence" in seen[0].error
|
||||||
|
# The traceback rides along in the logs, so a failure can be read back off
|
||||||
|
# the run rather than reproduced under `run --local`.
|
||||||
|
assert "Traceback" in seen[0].logs
|
||||||
|
assert "ValueError: no convergence" in seen[0].logs
|
||||||
|
|
||||||
|
|
||||||
def test_a_failing_observer_does_not_take_the_node_down():
|
def test_a_failing_observer_does_not_take_the_node_down():
|
||||||
|
|||||||
@@ -282,6 +282,53 @@ def test_ctrl_c_while_waiting_cancels_the_run(monkeypatch) -> None:
|
|||||||
assert cancelled == ["run-1"]
|
assert cancelled == ["run-1"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_failed_run_prints_what_each_node_said(monkeypatch, capsys) -> None:
|
||||||
|
"""Otherwise the traceback is stored and nothing at a terminal shows it."""
|
||||||
|
from contextlib import contextmanager
|
||||||
|
|
||||||
|
from fluksio.cli import _parser
|
||||||
|
from fluksio.sdk import cli
|
||||||
|
|
||||||
|
class FakeHandle:
|
||||||
|
id = "run-1"
|
||||||
|
status = "error"
|
||||||
|
result: dict[str, object] = {}
|
||||||
|
failures = [
|
||||||
|
{
|
||||||
|
"node": "study.train",
|
||||||
|
"error": "ValueError: no convergence",
|
||||||
|
"logs": 'Traceback (most recent call last):\n File "<node>"\n',
|
||||||
|
}
|
||||||
|
]
|
||||||
|
|
||||||
|
def wait(self, timeout: float = 0.0) -> "FakeHandle":
|
||||||
|
return self
|
||||||
|
|
||||||
|
class FakeClient:
|
||||||
|
def get_flow(self, name: str) -> dict[str, object]:
|
||||||
|
return {"definition": {"inputs": []}}
|
||||||
|
|
||||||
|
def submit(self, flow, params, seed=None, no_cache=False, cause="sdk"):
|
||||||
|
return FakeHandle()
|
||||||
|
|
||||||
|
def run(self, run_id: str) -> dict[str, object]:
|
||||||
|
return {"nodes": []}
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def fake_engine():
|
||||||
|
yield FakeClient()
|
||||||
|
|
||||||
|
monkeypatch.setattr(cli, "_engine_client", fake_engine)
|
||||||
|
|
||||||
|
args = _parser().parse_args(["run", "train", "--local", "--no-sync"])
|
||||||
|
assert cli.cmd_run(args, []) == 1
|
||||||
|
|
||||||
|
printed = capsys.readouterr().out
|
||||||
|
assert "study.train" in printed
|
||||||
|
assert "ValueError: no convergence" in printed
|
||||||
|
assert "Traceback (most recent call last):" in printed
|
||||||
|
|
||||||
|
|
||||||
def test_an_artifact_input_may_be_named_rather_than_pasted() -> None:
|
def test_an_artifact_input_may_be_named_rather_than_pasted() -> None:
|
||||||
"""The engine resolves either spelling; the CLI just stops mangling them."""
|
"""The engine resolves either spelling; the CLI just stops mangling them."""
|
||||||
import json
|
import json
|
||||||
|
|||||||
Reference in New Issue
Block a user