Skip to content

Commit f72f202

Browse files
glenrgordonclaude
andcommitted
fix: do not re-stringify a failed exception when building an internal error
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>
1 parent 9853d8b commit f72f202

2 files changed

Lines changed: 64 additions & 2 deletions

File tree

‎src/acp/connection.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,19 @@ async def _execute_request(self, message: dict[str, Any]) -> dict[str, Any]:
278278
method,
279279
exc_info=exc,
280280
)
281+
# Stringify once, before either branch: this is the recovery path for an
282+
# exception, so re-running ``str(exc)`` inside an ``except`` clause has no
283+
# sibling handler left and escapes ``_execute_request`` unanswered.
281284
try:
282-
data = json.loads(str(exc))
285+
text = str(exc)
283286
except Exception:
284-
data = {"details": str(exc)}
287+
# ``type(exc).__name__`` reads the class attribute and calls nothing on
288+
# the instance, unlike ``repr(exc)``, which is the same hazard again.
289+
text = f"<unprintable {type(exc).__name__}>"
290+
try:
291+
data = json.loads(text)
292+
except Exception:
293+
data = {"details": text}
285294
payload["error"] = RequestError.internal_error(data).to_error_obj()
286295
return payload
287296

‎tests/test_rpc.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,3 +975,56 @@ async def test_connection_init_under_eager_task_factory(server):
975975
await asyncio.sleep(0)
976976
assert conn._disconnected is False
977977
await conn.close()
978+
979+
980+
class _UnprintableError(Exception):
981+
"""Handler crash whose ``__str__`` raises, like a proxy whose backing state is gone."""
982+
983+
def __str__(self) -> str:
984+
raise RuntimeError("__str__ exploded")
985+
986+
987+
class _JsonMessageError(Exception):
988+
"""Handler crash whose message is itself a JSON document."""
989+
990+
def __str__(self) -> str:
991+
return json.dumps({"reason": "busy", "retryAfter": 5})
992+
993+
994+
async def _answer_request(server, exc: Exception) -> dict[str, Any]:
995+
"""Serve one request from a handler raising ``exc`` and return the response frame."""
996+
997+
async def handler(method: str, params: Any, is_notification: bool) -> Any:
998+
raise exc
999+
1000+
conn = Connection(handler, server.server_writer, server.server_reader)
1001+
try:
1002+
req = {"jsonrpc": "2.0", "id": 1, "method": "explode", "params": {}}
1003+
server.client_writer.write((json.dumps(req) + "\n").encode())
1004+
await server.client_writer.drain()
1005+
line = await asyncio.wait_for(server.client_reader.readline(), timeout=2)
1006+
finally:
1007+
await conn.close()
1008+
return cast("dict[str, Any]", json.loads(line))
1009+
1010+
1011+
@pytest.mark.asyncio
1012+
async def test_handler_exception_with_raising_str_still_answers(server):
1013+
# Regression: the internal-error recovery called str(exc) a second time inside its own
1014+
# except clause, so the retry escaped _execute_request and no reply was ever written.
1015+
resp = await _answer_request(server, _UnprintableError())
1016+
1017+
assert resp["id"] == 1
1018+
assert resp["error"]["code"] == -32603
1019+
# The type name is what distinguishes this from an ordinary crash with an empty message.
1020+
assert "_UnprintableError" in resp["error"]["data"]["details"]
1021+
1022+
1023+
@pytest.mark.asyncio
1024+
async def test_handler_exception_with_json_message_keeps_parsed_data(server):
1025+
# Pre-existing behaviour the stringify-once rewrite must not break.
1026+
resp = await _answer_request(server, _JsonMessageError())
1027+
1028+
assert resp["id"] == 1
1029+
assert resp["error"]["code"] == -32603
1030+
assert resp["error"]["data"] == {"reason": "busy", "retryAfter": 5}

0 commit comments

Comments
 (0)