From f72f202861eb9eb55f01c62696739b0b679d1316 Mon Sep 17 00:00:00 2001
From: Glen Gordon <18452241+glenrgordon@users.noreply.github.com>
Date: Thu, 5 Nov 2026 08:16:59 -0600
Subject: [PATCH] fix: do not re-stringify a failed exception when building an
 internal error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The internal-error recovery in `Connection._execute_request` guarded
`json.loads(str(exc))` with a `try`, then called `str(exc)` a second time
inside that `except` clause. An exception whose `__str__` raises therefore
made the recovery path re-run the call that had just failed, where no
sibling handler could catch it: the error propagated out of
`_execute_request`, `_run_request` re-raised at `handler.result()`, and no
reply was ever written. The peer waits on that request id forever.

Any exception whose `__str__` depends on state that is gone reaches this —
a lazy proxy, a closed cursor, a partially-initialised object.

Stringify once, before either branch, and fall back to
`f"<unprintable {type(exc).__name__}>"`, which reads the class attribute and
calls nothing on the instance. `repr(exc)` would be the same hazard again.

An exception whose message is valid JSON still takes the `json.loads`
branch; a test pins that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/acp/connection.py | 13 +++++++++--
 tests/test_rpc.py     | 53 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/acp/connection.py b/src/acp/connection.py
index 84d009cf..8c49689a 100644
--- a/src/acp/connection.py
+++ b/src/acp/connection.py
@@ -278,10 +278,19 @@ async def _execute_request(self, message: dict[str, Any]) -> dict[str, Any]:
                     method,
                     exc_info=exc,
                 )
+                # Stringify once, before either branch: this is the recovery path for an
+                # exception, so re-running ``str(exc)`` inside an ``except`` clause has no
+                # sibling handler left and escapes ``_execute_request`` unanswered.
                 try:
-                    data = json.loads(str(exc))
+                    text = str(exc)
                 except Exception:
-                    data = {"details": str(exc)}
+                    # ``type(exc).__name__`` reads the class attribute and calls nothing on
+                    # the instance, unlike ``repr(exc)``, which is the same hazard again.
+                    text = f"<unprintable {type(exc).__name__}>"
+                try:
+                    data = json.loads(text)
+                except Exception:
+                    data = {"details": text}
                 payload["error"] = RequestError.internal_error(data).to_error_obj()
         return payload
 
diff --git a/tests/test_rpc.py b/tests/test_rpc.py
index 43c37fd1..f5620f56 100644
--- a/tests/test_rpc.py
+++ b/tests/test_rpc.py
@@ -975,3 +975,56 @@ async def test_connection_init_under_eager_task_factory(server):
     await asyncio.sleep(0)
     assert conn._disconnected is False
     await conn.close()
+
+
+class _UnprintableError(Exception):
+    """Handler crash whose ``__str__`` raises, like a proxy whose backing state is gone."""
+
+    def __str__(self) -> str:
+        raise RuntimeError("__str__ exploded")
+
+
+class _JsonMessageError(Exception):
+    """Handler crash whose message is itself a JSON document."""
+
+    def __str__(self) -> str:
+        return json.dumps({"reason": "busy", "retryAfter": 5})
+
+
+async def _answer_request(server, exc: Exception) -> dict[str, Any]:
+    """Serve one request from a handler raising ``exc`` and return the response frame."""
+
+    async def handler(method: str, params: Any, is_notification: bool) -> Any:
+        raise exc
+
+    conn = Connection(handler, server.server_writer, server.server_reader)
+    try:
+        req = {"jsonrpc": "2.0", "id": 1, "method": "explode", "params": {}}
+        server.client_writer.write((json.dumps(req) + "\n").encode())
+        await server.client_writer.drain()
+        line = await asyncio.wait_for(server.client_reader.readline(), timeout=2)
+    finally:
+        await conn.close()
+    return cast("dict[str, Any]", json.loads(line))
+
+
+@pytest.mark.asyncio
+async def test_handler_exception_with_raising_str_still_answers(server):
+    # Regression: the internal-error recovery called str(exc) a second time inside its own
+    # except clause, so the retry escaped _execute_request and no reply was ever written.
+    resp = await _answer_request(server, _UnprintableError())
+
+    assert resp["id"] == 1
+    assert resp["error"]["code"] == -32603
+    # The type name is what distinguishes this from an ordinary crash with an empty message.
+    assert "_UnprintableError" in resp["error"]["data"]["details"]
+
+
+@pytest.mark.asyncio
+async def test_handler_exception_with_json_message_keeps_parsed_data(server):
+    # Pre-existing behaviour the stringify-once rewrite must not break.
+    resp = await _answer_request(server, _JsonMessageError())
+
+    assert resp["id"] == 1
+    assert resp["error"]["code"] == -32603
+    assert resp["error"]["data"] == {"reason": "busy", "retryAfter": 5}