fix: JSON-RPC protocol compliance + protocol tests

[ping-ke]

Summary

Brings the HTTP and WebSocket JSON-RPC servers into spec compliance for malformed input,
notifications, and batch requests, and adds a protocol test suite. Also hardens the JSON-RPC client
to validate response ids.

Related Issue

#985

Changes

• jsonrpc.py (HTTP/WS __handle): return -32700 Parse error on malformed JSON instead of
  swallowing the error and dispatching an empty dict. Drop the dead d = dict() fallback and guard
  non-dict payloads with isinstance.
• jsonrpc_server.py (dispatch): suppress responses for notifications (requests with no
  "id"), even on the error paths. Batch (JSON array) requests are intentionally still unsupported
  but now rejected cleanly with -32600 instead of crashing with an AttributeError / HTTP 500.
• jsonrpc_client.py: JsonRpcClient / AsyncJsonRpcClient now raise JsonRpcError(-32600)
  when the response id doesn't match the request id; also fixes a missing trailing newline.
• tests/test_jsonrpc.py: add TestJSONRPCProtocol — parse error, batch rejection,
  notification-no-response, error-uses-HTTP-200, and a success baseline (standalone master-free
  server on port 38391, raw POST for malformed/array bodies).

Files

quarkchain/cluster/jsonrpc.py, quarkchain/cluster/jsonrpc_server.py,
quarkchain/jsonrpc_client.py, quarkchain/cluster/tests/test_jsonrpc.py

[blockchaindevsh]

From Fable:

Review: PR #988 — fix: JSON-RPC protocol compliance + protocol tests

Verdict up front: Approve. All fixes are spec-correct, all prior reviewer comments are resolved at head 2afc221, and CI is green.

Overview

This PR (4 commits, base upgrade/py313-baseline) hardens the home-grown JSON-RPC layer that replaced the old jsonrpcserver 3.5.4 dependency. It fixes three classes of spec violations and adds a protocol-level test suite:

1. Malformed JSON — the HTTP server previously swallowed the json.loads failure (d = dict() fallback) and dispatched an empty dict, producing a misleading -32600 Invalid Request; the WS server raised InvalidParams, which escaped the async for
   loop and killed the connection, silently dropping every subscription on it (via the finally cleanup at jsonrpc.py:1527). Both now return a proper -32700 Parse error.
2. JSON arrays / scalars over HTTP — the base code called d.get("method", ...) on whatever json.loads returned, so a batch array crashed with AttributeError → HTTP 500. Now guarded with isinstance and rejected cleanly with -32600.
3. Notifications — dispatch() suppressed responses only on the success path; error paths still returned bodies. Per JSON-RPC 2.0, errors are now suppressed too.
4. Client hardening — JsonRpcClient/AsyncJsonRpcClient now validate that the response id matches the request id.

Correctness analysis

Each change was verified against the base (7f695c9) and head; the local working tree is byte-identical to the PR head for all four files.

• jsonrpc.py:523-533 (HTTP __handle) — parse error returns {"jsonrpc":"2.0","error":{-32700},"id":null} with HTTP 200. Correct per spec (id must be null when it can't be determined). The isinstance(d, dict) guard at line 535 fixes the real
  AttributeError/500 on arrays; non-dict payloads flow into dispatch(), whose pre-existing check (jsonrpc_server.py:95-96) yields a clean -32600.
• jsonrpc.py:1488-1496 (WS __handle) — sends -32700 and continues instead of raising, preserving the connection and its sub_ids state — the most user-visible fix. The isinstance guards on method/msg_id (lines 1497, 1503) prevent the same
  AttributeError for array/scalar frames over WS.
• jsonrpc_server.py:94-101, 141-156 (dispatch) — is_notification is hoisted above validation and both except branches return None for notifications. Two subtleties, both fine: non-dict payloads raise InvalidRequest before is_notification is
  set, so the client still gets the -32600 body; and an object lacking "id" with a bad jsonrpc version gets no response — spec-ambiguous, but matches what jsonrpcserver did. logger.exception(...) is correctly placed before the notification
  early-return (line 152), so crashing fire-and-forget handlers still leave a server-side trace.
• jsonrpc_client.py:31-34, 63-66 — the id check sits after the "error" check (the duplicate flagged in an earlier review comment was removed), so server errors carrying "id": null are no longer masked by a bogus "id mismatch". Verified in
  both sync and async clients.
• No caller regressions: WS dispatch callers already handle None responses, and the HTTP handler's asyncio.shield wrapping is untouched.

All four prior review comments from qzhodl (error-before-id ordering ×2, logging before notification return, missing WS test) are addressed at head.

Code quality

Minor nits, none blocking:

• logger.exception("... %s", locals().get("method", "")) works but is unidiomatic — pre-initializing method = "" before the try would be cleaner.
• The triple isinstance checks in WS __handle are repetitive; an early non-dict rejection would read better.
• Arrays are valid JSON-RPC (batches), so a "Batch requests not supported" message would be friendlier than the generic "Request must be object". The no-batch deviation itself is intentional and documented.

Test coverage

Good. TestJSONRPCProtocol covers parse error, batch rejection (no 500), notification-no-response, error-with-HTTP-200, and a success baseline using a standalone master-free server (the func.get binding trick in JSONRPCHttpServer.init
was verified to work). The WS test test_malformed_json_keeps_connection_alive verifies both the -32700 reply and that a subsequent subscribe on the same socket succeeds — exactly the regression that matters. Acceptable gaps:
array/notification frames over WS, explicit "id": null requests. Tests couldn't be run locally (no Python env with project deps), but CI runs them: all 4 checks green on head.

Performance / security

Neutral-to-positive: malformed input no longer produces unhandled exceptions, HTTP 500s, or torn-down WS subscription state — removes a trivial remote nuisance vector. Side effect: unparseable requests no longer increment self.counters, which
is fine since counters only meaningfully tracked parsed methods.

Optional follow-ups (non-blocking): pre-initialized method variable instead of locals().get, a "Batch requests not supported" message for arrays, and possibly HTTP 204 instead of an empty 200 body for notifications.

[blockchaindevsh]

From Fable:

PR #988 incremental review (2afc221 → 5548bac)

Changes: Only one new commit ("Improve JSON-RPC error messages and logging"), touching only jsonrpc_server.py — a direct response to the two non-blocking suggestions from the previous review:

1. Pre-initialized method variable (jsonrpc_server.py:95): dispatch now initializes method = "" at the top, and the internal-error log uses it directly, replacing the previously flagged locals().get("method", "") pattern. Verified
   that in that branch method can only be "" or an already-validated string — it will never log None or a stale value.
2. Explicit batch-request rejection (jsonrpc_server.py:98-99): JSON arrays now return "Batch requests not supported" (error code still -32600, id: null), conforming to what JSON-RPC 2.0 prescribes for an invalid batch. The WS path goes
   through the same dispatch, so it benefits automatically.

Status of prior feedback: Both suggestions have been addressed; the optional "HTTP 204 for notifications" suggestion was not adopted (existing tests pin the 200-with-empty-body behavior, which is acceptable); all of qzhodl's comments are
resolved at head.

CI: All 4 checks green.

Verdict: Approve. Only two purely cosmetic points remain (the log shows an empty-string method when extraction fails; the batch test could also assert the new error message) — neither is blocking.