From 9c7e879743cd295a24e491b7645aabc7e33810ea Mon Sep 17 00:00:00 2001 From: arelchan <204152633+arelchan@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:05:05 +0800 Subject: [PATCH 1/2] fix(tui_rpc): surface the cause of an internal_error, not just the code A -32603 reached the user as the bare line `error: [rpc -32603] internal_error`, with nothing about what failed or where to look. The cause was collected and then thrown away three times over: - the dispatcher dropped `RpcError.detail` whenever the raiser also passed `data`, which `_build_tui_agent_loop` always does; - `turn.send` emitted a latched init crash as `{code, message}` only, even though the ErrorEvent schema already has a `detail` field the front end renders; - the client formatted `[rpc ] `, where `message` is a fixed code name, and every call site prints `err.message`. So an AgentLoop that cannot start (a config the running branch cannot parse, for instance) produced an error that named neither the file nor the log. Fold `detail` into `error.data` in a shared `error_data` helper used by both the dispatcher and the turn events, pass the init-crash detail plus its `log_path` into the emitted event, and build the client-side message from `detail` / `exception_message` / `reason` with the log path appended. A multi-line cause (a config error listing each offending field) keeps its line breaks below the summary line. Co-authored-by: Claude (claude-opus-5) --- raven/tui_rpc/dispatcher.py | 8 ++--- raven/tui_rpc/errors.py | 14 ++++++++ raven/tui_rpc/methods/turn.py | 41 ++++++++++++++++++---- tests/test_tui_rpc_system.py | 22 +++++++++++- tests/test_tui_rpc_turn_send.py | 41 ++++++++++++++++++++++ ui-tui/src/__tests__/rpc.test.ts | 60 ++++++++++++++++++++++++++++++++ ui-tui/src/rpc/errors.ts | 34 +++++++++++++++++- 7 files changed, 207 insertions(+), 13 deletions(-) diff --git a/raven/tui_rpc/dispatcher.py b/raven/tui_rpc/dispatcher.py index 4b64e5bd..d09eddaf 100644 --- a/raven/tui_rpc/dispatcher.py +++ b/raven/tui_rpc/dispatcher.py @@ -26,6 +26,7 @@ METHOD_NOT_FOUND, PARSE_ERROR, RpcError, + error_data, ) Handler = Callable[[dict[str, Any]], Awaitable[dict[str, Any]]] @@ -106,10 +107,9 @@ async def dispatch(self, frame: dict[str, Any]) -> dict[str, Any]: "code": exc.code, "message": exc.message, } - if exc.data is not None: - err_payload["data"] = exc.data - elif exc.detail: - err_payload["data"] = {"detail": exc.detail} + payload_data = error_data(exc) + if payload_data is not None: + err_payload["data"] = payload_data return {"jsonrpc": "2.0", "id": frame_id, "error": err_payload} except SystemExit as exc: # Click/Typer can leak SystemExit even with standalone_mode=False; diff --git a/raven/tui_rpc/errors.py b/raven/tui_rpc/errors.py index 85be4b8d..5b9c6008 100644 --- a/raven/tui_rpc/errors.py +++ b/raven/tui_rpc/errors.py @@ -61,6 +61,20 @@ def message(self) -> str: return self.MESSAGE +def error_data(exc: RpcError) -> dict[str, Any] | None: + """Wire ``error.data`` for an exception: ``data`` with ``detail`` folded in. + + ``message`` is a fixed code name, so ``detail`` is the only place the cause + is spelled out; callers that set both must not lose it. Shared by the + dispatcher's error frames and the per-turn error events so both carry the + same context. + """ + data = dict(exc.data) if exc.data is not None else {} + if exc.detail: + data.setdefault("detail", exc.detail) + return data or None + + class SessionNotFoundError(RpcError): CODE = -32001 MESSAGE = "session_not_found" diff --git a/raven/tui_rpc/methods/turn.py b/raven/tui_rpc/methods/turn.py index e75fa42f..de9eeb3d 100644 --- a/raven/tui_rpc/methods/turn.py +++ b/raven/tui_rpc/methods/turn.py @@ -26,7 +26,7 @@ from raven.spine import ChatType, Media, Origin, Source, TurnHandle, TurnRequest from raven.spine.scheduler import Scheduler, SchedulerDrainingError -from raven.tui_rpc.errors import RpcError, TurnInProgressError +from raven.tui_rpc.errors import RpcError, TurnInProgressError, error_data from raven.tui_rpc.models import ( TurnCancelParams, TurnSendParams, @@ -134,16 +134,38 @@ def _resolve_model(parsed: TurnSendParams) -> str: # --------------------------------------------------------------------------- +def _build_error_detail(exc: RpcError) -> str | None: + """Human-readable cause of a latched init crash. + + ``message`` is only the code name (``internal_error``), so without this the + TUI shows a turn failing for no stated reason. ``log_path`` rides along + because an init crash is usually only fully diagnosable from the log. + """ + data = error_data(exc) or {} + detail = data.get("detail") or data.get("exception_message") + if not isinstance(detail, str) or not detail.strip(): + return None + log_path = data.get("log_path") + if isinstance(log_path, str) and log_path.strip(): + return f"{detail.strip()} (details in {log_path.strip()})" + return detail.strip() + + async def _emit_start_then_error( - emitter: SubscriptionEmitter, session_key: str, turn_id: str, code: int, message: str + emitter: SubscriptionEmitter, + session_key: str, + turn_id: str, + code: int, + message: str, + detail: str | None = None, ) -> None: # message.start first so the front-end has a turn to clear, then the error # clears it (its onError resets turnId) — same shape the old per-turn task used. await emitter.emit(session_key, {"type": "message.start", "payload": {"turn_id": turn_id}}) - await emitter.emit( - session_key, - {"type": "error", "payload": {"code": code, "message": message, "reason": "internal"}}, - ) + payload: dict[str, Any] = {"code": code, "message": message, "reason": "internal"} + if detail: + payload["detail"] = detail + await emitter.emit(session_key, {"type": "error", "payload": payload}) async def turn_send( @@ -182,7 +204,12 @@ async def turn_send( if emitter is not None: if build_error is not None: await _emit_start_then_error( - emitter, parsed.session_key, turn_id, build_error.code, build_error.message + emitter, + parsed.session_key, + turn_id, + build_error.code, + build_error.message, + _build_error_detail(build_error), ) else: await _emit_start_then_error(emitter, parsed.session_key, turn_id, -32008, "model_not_available") diff --git a/tests/test_tui_rpc_system.py b/tests/test_tui_rpc_system.py index a8660bb8..3f58026e 100644 --- a/tests/test_tui_rpc_system.py +++ b/tests/test_tui_rpc_system.py @@ -17,7 +17,7 @@ import pytest from raven.tui_rpc.dispatcher import Dispatcher -from raven.tui_rpc.errors import ConfigValidationError +from raven.tui_rpc.errors import ConfigValidationError, InternalError from raven.tui_rpc.methods.system import ( register_system_methods, system_hello, @@ -162,6 +162,26 @@ async def boom(params: dict) -> dict: assert "traceback_tail" in resp["error"]["data"] +async def test_dispatcher_keeps_detail_alongside_structured_data(): + # `message` is only a code name, so dropping `detail` when a raiser also set + # `data` leaves the client with nothing to show. Both must reach the wire. + d = Dispatcher() + + async def boom(params: dict) -> dict: + raise InternalError( + detail="Config at ~/.raven/config.json fails schema validation", + data={"reason": "tui_init_crash", "log_path": "~/.raven/logs/tui.log"}, + ) + + d.register("test.boom", boom) + resp = await d.dispatch({"jsonrpc": "2.0", "id": 7, "method": "test.boom", "params": {}}) + + assert resp["error"]["code"] == -32603 + assert resp["error"]["data"]["detail"] == "Config at ~/.raven/config.json fails schema validation" + assert resp["error"]["data"]["reason"] == "tui_init_crash" + assert resp["error"]["data"]["log_path"] == "~/.raven/logs/tui.log" + + async def test_dispatcher_parse_response_id_echoed(): d = _build_dispatcher() frame = { diff --git a/tests/test_tui_rpc_turn_send.py b/tests/test_tui_rpc_turn_send.py index 181153d3..b545a540 100644 --- a/tests/test_tui_rpc_turn_send.py +++ b/tests/test_tui_rpc_turn_send.py @@ -188,6 +188,47 @@ class _BuildErr(RpcError): assert emitter.emitted[-1][1]["payload"]["code"] == -32603 +async def test_turn_send_emits_the_build_error_cause_not_just_its_code() -> None: + # -32603 internal_error names no cause on its own; the init crash detail and + # the log path are what make the failure diagnosable in the transcript. + class _BuildErr(RpcError): + CODE = -32603 + MESSAGE = "internal_error" + + emitter = FakeEmitter() + build_error = _BuildErr( + "Config at ~/.raven/config.json fails schema validation", + {"reason": "tui_init_crash", "log_path": "~/.raven/logs/tui.log"}, + ) + await turn_send( + {"session_key": "tui:default", "content": "x"}, + emitter=emitter, + scheduler=None, + build_error=build_error, + ) + + payload = emitter.emitted[-1][1]["payload"] + assert payload["detail"] == ( + "Config at ~/.raven/config.json fails schema validation (details in ~/.raven/logs/tui.log)" + ) + + +async def test_turn_send_omits_detail_when_the_build_error_has_no_cause() -> None: + class _BuildErr(RpcError): + CODE = -32603 + MESSAGE = "internal_error" + + emitter = FakeEmitter() + await turn_send( + {"session_key": "tui:default", "content": "x"}, + emitter=emitter, + scheduler=None, + build_error=_BuildErr(), + ) + + assert "detail" not in emitter.emitted[-1][1]["payload"] + + # --- Params validation --- diff --git a/ui-tui/src/__tests__/rpc.test.ts b/ui-tui/src/__tests__/rpc.test.ts index 7980093a..ae8cd7b6 100644 --- a/ui-tui/src/__tests__/rpc.test.ts +++ b/ui-tui/src/__tests__/rpc.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js' +import { RpcError, rpcErrorFromFrame, SessionNotFoundError } from '../rpc/errors.js' describe('asRpcResult', () => { it('keeps plain object payloads', () => { @@ -25,3 +26,62 @@ describe('rpcErrorMessage', () => { expect(rpcErrorMessage({ code: 500 })).toBe('request failed') }) }) + +describe('rpcErrorFromFrame', () => { + it('keeps the bare code name when the frame carries no context', () => { + const err = rpcErrorFromFrame({ code: -32603, message: 'internal_error' }) + expect(err.message).toBe('[rpc -32603] internal_error') + }) + + it('surfaces the cause the server put in data, plus where to read more', () => { + const err = rpcErrorFromFrame({ + code: -32603, + message: 'internal_error', + data: { + reason: 'tui_init_crash', + detail: 'Config at ~/.raven/config.json fails schema validation', + log_path: '~/.raven/logs/tui.log' + } + }) + expect(err.message).toBe( + '[rpc -32603] internal_error: Config at ~/.raven/config.json fails schema validation\n' + + '(details in ~/.raven/logs/tui.log)' + ) + }) + + it('reads exception_message and reason when detail is absent', () => { + expect( + rpcErrorFromFrame({ code: -32603, message: 'internal_error', data: { exception_message: 'boom' } }).message + ).toBe('[rpc -32603] internal_error: boom') + expect(rpcErrorFromFrame({ code: -32603, message: 'internal_error', data: { reason: 'uncaught' } }).message).toBe( + '[rpc -32603] internal_error: uncaught' + ) + }) + + it('keeps a multi-line cause readable below the summary', () => { + const err = rpcErrorFromFrame({ + code: -32011, + message: 'config_validation_error', + data: { detail: '2 validation errors\nsubagents: extra inputs are not permitted' } + }) + expect(err.message).toBe( + '[rpc -32011] config_validation_error:\n2 validation errors\nsubagents: extra inputs are not permitted' + ) + }) + + it('ignores non-object and blank data without losing the code name', () => { + for (const data of [undefined, null, ['detail'], 'detail', { detail: ' ' }, { detail: 7 }]) { + expect(rpcErrorFromFrame({ code: -32603, message: 'internal_error', data }).message).toBe( + '[rpc -32603] internal_error' + ) + } + }) + + it('still selects the typed subclass and exposes raw data', () => { + const err = rpcErrorFromFrame({ code: -32001, message: 'session_not_found', data: { detail: 'no such key' } }) + expect(err).toBeInstanceOf(SessionNotFoundError) + expect(err).toBeInstanceOf(RpcError) + expect(err.code).toBe(-32001) + expect(err.data).toEqual({ detail: 'no such key' }) + }) +}) diff --git a/ui-tui/src/rpc/errors.ts b/ui-tui/src/rpc/errors.ts index a2d93b2d..8cfe9348 100644 --- a/ui-tui/src/rpc/errors.ts +++ b/ui-tui/src/rpc/errors.ts @@ -7,13 +7,45 @@ import type { JsonRpcErrorObject } from './generated.js' +/** Fields the server puts in `error.data` to explain a failure (see + * `raven/tui_rpc/errors.py` and `_build_tui_agent_loop`). `frame.message` is a + * fixed code name like `internal_error`, so without these the user is told + * nothing actionable. */ +const DETAIL_KEYS = ['detail', 'exception_message', 'reason'] as const + +const readString = (data: Record, key: string): string | undefined => { + const value = data[key] + return typeof value === 'string' && value.trim() ? value.trim() : undefined +} + +/** `[rpc -32603] internal_error: (see ~/.raven/logs/tui.log)`. + * Callers render `err.message` directly, so the cause has to live there. */ +export function formatRpcError(frame: JsonRpcErrorObject): string { + let text = `[rpc ${frame.code}] ${frame.message}` + if (typeof frame.data !== 'object' || frame.data === null || Array.isArray(frame.data)) { + return text + } + const data = frame.data as Record + const detail = DETAIL_KEYS.map(key => readString(data, key)).find(Boolean) + // A one-line detail reads inline; a multi-line one (a config error listing + // every offending field, say) keeps its shape below the summary. + if (detail) { + text += detail.includes('\n') ? `:\n${detail}` : `: ${detail}` + } + const logPath = readString(data, 'log_path') + if (logPath) { + text += `\n(details in ${logPath})` + } + return text +} + /** Base class for all JSON-RPC error responses surfaced to callers. */ export class RpcError extends Error { readonly code: number readonly data: unknown constructor(frame: JsonRpcErrorObject) { - super(`[rpc ${frame.code}] ${frame.message}`) + super(formatRpcError(frame)) this.name = 'RpcError' this.code = frame.code this.data = frame.data From e8417b22338f79eb6531c76be993f29ce93ef791 Mon Sep 17 00:00:00 2001 From: arelchan <204152633+arelchan@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:08:24 +0800 Subject: [PATCH 2/2] fix(tui_rpc): put the log pointer where the renderer cannot cut it Review caught that the pointer did not survive to the user in the case that motivated it. The transcript renders the first line of `detail` and nothing more (ui-tui/src/app/chatStream.ts), and the crash this detail exists for -- a config the running build cannot parse -- raises a ValidationError whose str() is multi-line: 7 lines for two bad fields. Appended, `(details in ~/.raven/logs/tui.log)` landed on the last line, so the user saw `2 validation errors for RavenConfig` and no path. Verified before changing anything. Prepend it instead. Single-line causes read the same either way, and the multi-line ones now keep both halves in the rendered slice. The existing test asserted the whole string, which a trailing pointer passes just as well; the new one asserts what the renderer actually keeps. Co-authored-by: Claude (claude-opus-5) --- raven/tui_rpc/methods/turn.py | 9 ++++++++- tests/test_tui_rpc_turn_send.py | 26 +++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/raven/tui_rpc/methods/turn.py b/raven/tui_rpc/methods/turn.py index de9eeb3d..efec59c9 100644 --- a/raven/tui_rpc/methods/turn.py +++ b/raven/tui_rpc/methods/turn.py @@ -140,6 +140,13 @@ def _build_error_detail(exc: RpcError) -> str | None: ``message`` is only the code name (``internal_error``), so without this the TUI shows a turn failing for no stated reason. ``log_path`` rides along because an init crash is usually only fully diagnosable from the log. + + It rides in *front* of the cause, not behind it: the TUI renders the first + line of this string and nothing else (``chatStream.ts``), and the crash this + exists for -- a config the running build cannot parse -- raises a + ``ValidationError`` whose ``str()`` is always multi-line (7 lines for two bad + fields). Appended, the pointer landed on the last line and never reached + anyone; the one case that needs the log was the one case that lost it. """ data = error_data(exc) or {} detail = data.get("detail") or data.get("exception_message") @@ -147,7 +154,7 @@ def _build_error_detail(exc: RpcError) -> str | None: return None log_path = data.get("log_path") if isinstance(log_path, str) and log_path.strip(): - return f"{detail.strip()} (details in {log_path.strip()})" + return f"(details in {log_path.strip()}) {detail.strip()}" return detail.strip() diff --git a/tests/test_tui_rpc_turn_send.py b/tests/test_tui_rpc_turn_send.py index b545a540..9a8aa55a 100644 --- a/tests/test_tui_rpc_turn_send.py +++ b/tests/test_tui_rpc_turn_send.py @@ -209,10 +209,34 @@ class _BuildErr(RpcError): payload = emitter.emitted[-1][1]["payload"] assert payload["detail"] == ( - "Config at ~/.raven/config.json fails schema validation (details in ~/.raven/logs/tui.log)" + "(details in ~/.raven/logs/tui.log) Config at ~/.raven/config.json fails schema validation" ) +async def test_the_log_path_survives_the_transcript_renderer_on_a_multiline_cause() -> None: + # The TUI renders the first line of `detail` and nothing else. The crash this + # detail exists for is an unparseable config, whose ValidationError str() runs + # to several lines -- so a trailing pointer was dropped for exactly the case + # that needs it. Asserts the surviving slice, not just the whole string. + class _BuildErr(RpcError): + CODE = -32603 + MESSAGE = "internal_error" + + emitter = FakeEmitter() + multiline = "2 validation errors for RavenConfig\nagents.defaults.model\n Input should be a valid string" + await turn_send( + {"session_key": "tui:default", "content": "x"}, + emitter=emitter, + scheduler=None, + build_error=_BuildErr(multiline, {"reason": "tui_init_crash", "log_path": "~/.raven/logs/tui.log"}), + ) + + detail = emitter.emitted[-1][1]["payload"]["detail"] + rendered = detail.split("\n")[0][:200] # ui-tui/src/app/chatStream.ts + assert "~/.raven/logs/tui.log" in rendered + assert "2 validation errors for RavenConfig" in rendered + + async def test_turn_send_omits_detail_when_the_build_error_has_no_cause() -> None: class _BuildErr(RpcError): CODE = -32603