[glenrgordon]

Summary

A response payload is attributed to a request id before it is encoded, and json.dumps
runs inside transport.send. So any payload the serialiser refuses makes the TypeError
escape the response task, no reply is ever written, and the peer waits on that id forever.

Two routes reach this today, neither needing a hostile input:

• A handler returning a raw value that is not JSON-native, e.g. {"x": {1, 2}}. A
  BaseModel result is safe, because model_dump(mode="json", ...) coerces even an Any
  field holding a set into a list. A raw return value was assigned unchecked.
• A handler raising RequestError(..., data=...), whose data is typed Any and passed
  straight through to to_error_obj(). This is the SDK's own public way to return a
  structured protocol error, and Any is exactly where a caller puts "whatever object I had
  in hand".

The fix gates the finished payload once, at the end of _execute_request:

try:
    json.dumps(payload)
except Exception as exc:
    logging.exception("Response for method=%s could not be encoded", method, exc_info=exc)
    payload.pop("result", None)
    payload["error"] = RequestError.internal_error({"details": str(exc)}).to_error_obj()

Ten lines including comments, in one file, covering every payload shape rather than the two
routes that happen to be known. json and logging are already imported, so there is no new
import and nothing for deptry to say.

str(exc) is safe here specifically because exc comes from json.dumps: its message is
built from a type name (Object of type set is not JSON serializable), not from the offending
object, and the serialiser never calls repr on it.

Why the gate must use json.dumps

The natural implementation is to_jsonable_python — pydantic's serialiser, already a
dependency. It is not a faithful gate: without a fallback it coerces a set to a list and
bytes to a str and raises only on a genuinely unknown type, so it would pass payloads
json.dumps then refuses. That is a check which reports success and changes nothing. All the
transports encode with json.dumps (task/sender.py, _sse.py, ws/client.py, differing
only in separators), so the gate does too. Verified by substitution: built on
to_jsonable_python, the raw-result test fails.

Why the gate is not at the send

This is the part worth agreeing with before the fix:

A transport failure means the peer is unreachable — a closed pipe, a dropped socket.
Propagating it unanswered is right, because there is nobody left to answer, and
test_response_send_failure_is_not_mapped_to_a_handler_error asserts exactly that. An
encoding failure is different: the peer is reachable and waiting on a request id, and
the payload cannot be represented. The fault is ours.

Catching (TypeError, ValueError) around await self._transport.send(payload) would conflate
the two. It would also convert a transport implementation's own unrelated TypeError into a
-32603 reported to the peer as a protocol error, masking exactly the bug you want to see.
Gating inside _execute_request keeps those apart at the cost of one serialisation, and
tests/test_request_error_logging.py is untouched by this PR.

Also rejected: coercing the payload with fallback=str before sending. That ends the silence
by substituting a lie — {"result": {"x": "{1, 2}"}} arrives under a result key,
indistinguishable from a genuine answer, and the peer acts on it. An error is self-describing;
a mangled success is not. The gate drops result and replaces it with an error for the same
reason.

The cost, stated plainly

One extra json.dumps of the payload per response, including the BaseModel path that is
already known-safe: measured at ~1 µs for a small payload, ~11 µs at 10 KB, ~1.2 ms at 1 MB on
this machine. A narrower per-site check would avoid that on the BaseModel path at the cost of
two separate checks, more lines, and no guarantee about payload shapes nobody has written yet.
Happy to switch to the narrow form if the hot-path cost matters more than the coverage — the
per-site version is written and tested, and I can push it as an alternative.

Related issues

None filed yet. Opened as a draft so the reproduction and the fix can be read together; happy
to file a tracking issue first if maintainers prefer that order.

fc545d6 fix: align JSON-RPC error handling across transports (#174) is in main and does
not address either route — worth saying, since it is adjacent work a maintainer will recall.

Testing

Three tests added in tests/test_rpc.py, all driving a real Connection over a socket pair so
json.dumps runs where it runs in production:

• test_unserialisable_raw_result_answers_the_peer
• test_unserialisable_request_error_data_answers_the_peer
• test_base_model_result_with_a_set_is_coerced_and_still_answers

The first two fail on unmodified main by timing out — TimeoutError at
server_reader.readline(), never reaching an assertion, because no line is ever written. That
is the defect: not a wrong value, an absent reply. Each logs the real site on the way down:

  File "src/acp/connection.py", line 252, in _run_request
    await self._transport.send(payload)
  File "src/acp/task/sender.py", line 29, in send
    data = (json.dumps(payload, separators=(",", ":")) + "\n").encode("utf-8")
TypeError: Object of type set is not JSON serializable

The third passes before this change as well as after, deliberately. It pins the
pre-existing pydantic behaviour that makes a BaseModel result safe; if model_dump(mode="json")
ever stops coercing, that test says so. Also probed: a BaseModel whose Any field holds a
plain non-JSON-native object raises PydanticSerializationError inside the existing try and
becomes a -32603 already.

tests/test_request_error_logging.py is unmodified and passes, including
test_response_send_failure_is_not_mapped_to_a_handler_error. If this change had needed to edit
that file, it would have drifted into the send path and the reasoning above would be void.

Environment: Windows 11, CPython 3.13, uv, mingw32-make.

• make check: 2 ty diagnostics, both pre-existing unresolved-import for hypercorn in
  examples/http_server.py (undeclared in pyproject.toml and uv.lock; the target's
  --exclude "examples/*.py" does not match on this platform). All 10 prek hooks pass,
  ruff check and ruff format included. No new diagnostics.
• make test: 1 failed, 492 passed, 2 skipped — 489 plus these three tests. The one failure
  is a pre-existing Windows CRLF line-ending artifact in
  tests/real_user/test_stdio_limits.py (assert 71682 == 71681). A second test in that file
  is load-sensitive and fails intermittently here on unmodified main too. CI is the
  authority on the clean numbers.

Docs & screenshots

None needed. Nothing under docs/ describes the error payload, RequestError.data, or the
handler return contract — grepped for invalid_params, -32602 and RequestError with no
hits — so there is no page to update. The behaviour change runs in one direction only: a
handler that previously stranded its caller now gets a -32603 whose details name the
encoding failure. No handler that worked before behaves differently.

Checklist

• Conventional Commit title.
• Tests cover the change.
• Docs/examples updated when behaviour is user-facing — explained above.
• No schema regeneration; make gen-all not involved.