Skip to content

fix(jsonrpc): normalize error responses and fatal handling - #6

Open
waynercheung wants to merge 1 commit into
developfrom
feat/jsonrpc-error-sanitization
Open

fix(jsonrpc): normalize error responses and fatal handling#6
waynercheung wants to merge 1 commit into
developfrom
feat/jsonrpc-error-sanitization

Conversation

@waynercheung

@waynercheung waynercheung commented Aug 29, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Replaces jsonrpc4j's unhandled-exception fallback with spec-defined responses and stops converting fatal errors into JSON-RPC replies.

Resolver (JsonRpcErrorResolver):

  • Unmapped non-fatal exceptions -> -32603 "Internal error" with no data. Logging is bounded by (RPC method, exception class): the first occurrence is WARN with the Throwable; repeats are DEBUG without the Throwable or exception message.
  • Mapped exceptions get a message precedence of annotation > exception message > per-code default (Invalid Request / Method not found / Invalid params / Internal error), so message is never null. data is exception data > annotation data; jsonrpc4j's ErrorData(exceptionClass, message) default is gone.
  • VirtualMachineError, ThreadDeath, LinkageError and java-tron's TronError found anywhere on the cause chain (cycle-safe) are rethrown as the actual cause instead of being answered.
  • net_version / eth_chainId keep their documented -32001 through an explicit mapping ("Chain identity unavailable", data "{}"); ethChainId() keeps the cause and logs failure-state transitions (first failure WARN, recovery INFO, no repeated WARN during one outage).
  • ExecutionException / InterruptedException on the asynchronous log query get a fixed "Internal error" message; LogBlockQuery logs the cause and restores the interrupt flag.

Servlet (JsonRpcServlet):

  • Single requests go through handleRequest(InputStream, OutputStream) instead of handle(request, response), whose catch (Throwable) would swallow the rethrown fatal error. Single and batch dispatch map escaped RuntimeException or declared IOException to -32603 when a response is appropriate.
  • An outer doPost guard best-effort commits a zero-length HTTP 500 before rethrowing an escaped Error, so Jetty does not normally render its detail-bearing default error page. The guard never replaces the original fatal if cleanup fails; resource exhaustion may still close the connection instead of delivering the 500. This propagation does not itself terminate the process.
  • HTTP 200 and application/json-rpc are set explicitly at normal JSON-RPC servlet exits; the custom HttpStatusCodeProvider configuration and the now-unused CachedBodyRequestWrapper are removed.
  • Request envelope types are validated before dispatch. Boolean / object / array IDs get -32600 "Invalid Request" with id: null. Non-null scalar params also gets -32600; a valid id is echoed, while a missing, null or invalid id becomes null. In a batch only the offending element gets the error and the other elements still execute. An explicit id: null on an otherwise valid request is not rejected by these checks; its final semantics, and whether to reject params: null, are decided with [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) tronprotocol/java-tron#6676.
  • The single-request catch-all preserves a valid request id and answers a valid notification with an empty 200 body.

Why are these changes required?

An exception without an @JsonRpcErrors mapping currently produces (Java 8):

{"jsonrpc":"2.0","id":1,"error":{"code":-32001,"message":null,"data":"java.lang.NullPointerException"}}

message: null violates JSON-RPC 2.0 section 5.1 (on Java 17 it becomes a helpful-NPE string echoing internal class and method names), data exposes internal types, and -32001 is the code the public error catalog documents for the chain identity lookup, so callers cannot tell their own bad input from a node failure. Fatal errors such as OutOfMemoryError, StackOverflowError and TronError were converted into ordinary error responses, masking a process-level failure; after propagation, the servlet now best-effort commits a detail-free 500 rather than allowing Jetty's default error page to render the Throwable. Invalid request ID types produced HTTP 200 with an empty body for single requests. Scalar params had the same result after a registered method reached argument matching; an unknown method returned -32601 before inspecting params. In a batch, framework exceptions produced only -32603 with id: null and stopped further processing.

-32603 is the Internal error defined by the specification and matches Besu's RpcErrorType.INTERNAL_ERROR classification. Rejecting Boolean IDs is deliberately stricter than go-ethereum, following section 4 (String / Number / Null). Section 4.2 requires structured params; java-tron classifies a non-null scalar as -32600 at the request-envelope layer, matching Besu's error-code classification, while geth classifies it as -32602 during method-argument parsing. For this malformed shape without an id, java-tron returns id: null, while geth sends no response. Full analysis and reproduction steps are in tronprotocol#6941.

Request-envelope validation intentionally precedes method lookup: an unknown method with scalar params changes from -32601 to -32600, while the same method with valid params: [] remains -32601. A malformed request without an id is not a valid notification; valid notifications remain response-free.

This PR has been tested by:

  • Unit Tests

  • Manual Testing

  • JsonRpcErrorResolverTest (9 tests) - mapped code / data priority, message precedence and per-code defaults, sanitized -32603 for unmapped exceptions and a null method, bounded WARN/DEBUG logging by method and exception class, fatal error propagation including TronError (direct, wrapped, cyclic cause chain).

  • JsonRpcErrorSanitizationIntegrationTest (19 tests) - through a real JsonRpcServer and JsonRpcServlet: unmapped exceptions sanitized on the wire, four fatal categories escaping the server, servlet best-effort empty-500 handling, fixed messages for ExecutionException / InterruptedException, chain identity keeping -32001 with sanitized details, business messages preserved, batch isolation, transport contracts, scalar-params and request-ID rules, and real dispatch of supported params shapes.

  • JsonRpcDispatchContractTest (8 tests) - characterization of jsonrpc4j 1.6 dispatch (arity, null array elements, overload selection, scalar params) so a later framework upgrade cannot change behavior silently.

  • JsonRpcServletTest (39 tests) - request ID and scalar-params validation, exact pass-through of supported shapes, batch isolation, catch-all ID preservation, declared IOException handling for single/batch/notification requests, and proof that a fatal Error is rethrown after a bare 500 without cleanup failures replacing it.

  • LogBlockQueryFailureTest (2 tests) - cause logging and interrupt-flag restoration.

  • TronJsonRpcImplChainIdentityTest (1 test) - one WARN per failure episode, recovery INFO, and a new WARN after recovery.

  • JsonRpcServletJettyTest (1 test) - real embedded Jetty wiring verifies that a fatal marker and exception type do not appear in the HTTP response; an empty 500 or a closed connection is accepted.

CachedBodyRequestWrapperTest is removed together with the class. A no-build-cache JDK 17 (arm64) run passed 28 test classes / 278 tests (0 failures, 0 errors, 0 skipped); the seven focused classes above contain 79 tests. checkstyleMain, checkstyleTest, and git diff --check also pass.

Compatibility

Breaking, limited to observable failure-handling paths; no request that succeeds today starts failing.

Case Before After
Unmapped exception -32001, exception message (may be null), data = class name -32603 "Internal error", no data
net_version / eth_chainId failure -32001, underlying message, data = class name -32001 "Chain identity unavailable", data "{}"
ExecutionException / InterruptedException -32000, cause toString() / null -32000 "Internal error"
Boolean / object / array request ID Single request: HTTP 200, empty body; batch: only -32603 / id: null, then processing stops -32600 "Invalid Request", id: null; batch siblings continue
Non-null scalar params After a registered method is selected: single request is HTTP 200 with an empty body; batch returns only -32603 / id: null and stops. An unknown method returns -32601 before checking params. -32600 "Invalid Request", no data; valid id preserved, otherwise id: null; batch siblings continue. Envelope validation precedes method lookup, so unknown + scalar changes to -32600, while unknown + params: [] stays -32601.
Fatal Error (VirtualMachineError / ThreadDeath / LinkageError / TronError) converted into -32001 / -32000 propagates after a best-effort empty HTTP 500; the connection may close instead, and a batch loses accumulated results
handleRequest throws IOException hidden by the old servlet-level entry point with a valid ID, HTTP 200 / -32603; a notification remains response-free

Unchanged: successful responses, HTTP 200 whenever a normal JSON-RPC response is produced, existing dispatch and method validation for missing / null / Array / Object params, data "{}" on the 62 existing mapped errors, deliberate business messages such as "filter not found", gRPC and non-JSON-RPC HTTP APIs. Fatal and genuine transport failures are not normal JSON-RPC responses and do not carry an HTTP-200 guarantee.

Before merge:

  • The four affected entries of the public JSON-RPC error catalog (JSON_RPC_UNDERLYING_INTERNAL_ERROR, JSON_RPC_SERVLET_INTERNAL_ERROR, JSON_RPC_EXECUTION_ERROR, JSON_RPC_INTERRUPTED) need a documentation-en PR; the table is generated from x-tron-error-model in docs/api/openrpc.json.
  • Confirm that gateway error mappings, the official SDKs and monitoring rules do not depend on the old catch-all -32001 behavior or on the previous chain-identity message / data, and that their error classification accounts for the new -32603 responses.
  • No landing-order dependency on [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) tronprotocol/java-tron#6676; whichever change lands second rebases (see Extra details).

Follow up

  • Validation of the jsonrpc and method members, whether to reject params: null, and the final semantics of an explicit id: null on an otherwise valid request belong to [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) tronprotocol/java-tron#6676, which overlaps this PR in JsonRpcServlet and the TronJsonRpc annotation blocks (see Extra details).
  • jsonrpc4j's precision loss when round-tripping large integer or high-precision numeric request IDs is a separate compatibility follow-up; servlet-generated errors in this PR preserve the original JsonNode ID.
  • Unify the wording of the five mapped -32000 catch sites in eth_call / eth_estimateGas / buildTransaction.
  • jsonrpc4j 1.6 -> 1.7 upgrade, fixing the parameter type mismatch that returns -32700 and loses the request id.
  • Container-wide sanitization of non-413 Jetty error pages remains a separate HTTP-layer hardening topic; this PR protects only Errors that escape JsonRpcServlet.doPost, on a best-effort basis.

Extra details

maxResponseSize continues to limit dispatched response accumulation. As on existing servlet-generated error paths, protocol error envelopes are still emitted and may make the final body exceed that threshold. Request-body and token limits, together with the batch-size limit when enabled, bound this behavior; redefining the threshold as a hard final-body cap is out of scope for this PR.

An object with scalar params and no id is malformed rather than a valid notification, so it receives -32600 with id: null. Envelope validation also intentionally precedes method lookup; clients probing method availability should use a structurally valid params array or object.

This PR overlaps the request-envelope validation planned in tronprotocol#6676 in JsonRpcServlet and the eth_getLogs @JsonRpcErrors block of TronJsonRpc. There is no dependency between the two: this PR can land first and tronprotocol#6676 can build on the pre-dispatch checks added here; if tronprotocol#6676's PR lands first, this one will be rebased.

Pre-submit checklist:

  • Google Java Style; Checkstyle passes on main and test sources
  • No debug code, temporary comments or TODOs
  • No numeric computation or narrowing casts introduced
  • Logging: unmapped exceptions are WARNed once per method/type and repeated only at DEBUG; chain identity logs failure-state transitions; async log-query failure/interruption retain their call-site WARNs; nothing logs on the normal request path
  • No DB, consensus, config or dependency changes
  • Comments explain why handleRequest is required and why the chain-identity cause must be retained

Closes tronprotocol#6941
Refs tronprotocol#6676

Replace jsonrpc4j's unhandled -32001 fallback with a fixed -32603
response so unmapped exception types and raw messages no longer reach
clients. Keep the documented -32001 contract for net_version and
eth_chainId through explicit mappings with fixed message and data.

Propagate VirtualMachineError, ThreadDeath, LinkageError, and TronError
through the JSON-RPC boundary. Before rethrowing, make a best-effort
attempt to commit an empty HTTP 500 so Jetty does not render exception
details. Preserve the original fatal error even if cleanup fails.

Route single requests through handleRequest because the servlet handle
API catches Throwable. Convert escaped RuntimeException and IOException
instances into -32603 responses when a response is appropriate.

Bound unmapped-exception logging by RPC method and exception type, and
log chain-identity failures on state transitions. Keep complete causes
for the first diagnostic event and restore interrupted status for
asynchronous log queries.

Reject Boolean, object, and array request IDs before dispatch with
-32600 and id:null. Reject non-null scalar params with -32600, preserve
valid IDs, and isolate invalid batch elements so their siblings run.
Missing, null, array, and object params keep their existing semantics.

Remove the obsolete request replay wrapper and HTTP status provider.
Add resolver, servlet, embedded-Jetty, chain-identity, asynchronous
failure, dispatch-contract, and request-envelope regression coverage.

Add a package-private server injection seam for servlet tests without
changing production initialization.
@waynercheung
waynercheung force-pushed the feat/jsonrpc-error-sanitization branch from 993c3dc to 65da80f Compare September 2, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Standardize JSON-RPC error mapping and exception boundaries

1 participant