fix(jsonrpc): normalize error responses and fatal handling - #6
Open
waynercheung wants to merge 1 commit into
Open
fix(jsonrpc): normalize error responses and fatal handling#6waynercheung wants to merge 1 commit into
waynercheung wants to merge 1 commit into
Conversation
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
force-pushed
the
feat/jsonrpc-error-sanitization
branch
from
September 2, 2026 09:46
993c3dc to
65da80f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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):-32603 "Internal error"with nodata. 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.Invalid Request/Method not found/Invalid params/Internal error), somessageis nevernull.datais exception data > annotation data; jsonrpc4j'sErrorData(exceptionClass, message)default is gone.VirtualMachineError,ThreadDeath,LinkageErrorand java-tron'sTronErrorfound anywhere on the cause chain (cycle-safe) are rethrown as the actual cause instead of being answered.net_version/eth_chainIdkeep their documented-32001through 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/InterruptedExceptionon the asynchronous log query get a fixed"Internal error"message;LogBlockQuerylogs the cause and restores the interrupt flag.Servlet (
JsonRpcServlet):handleRequest(InputStream, OutputStream)instead ofhandle(request, response), whosecatch (Throwable)would swallow the rethrown fatal error. Single and batch dispatch map escapedRuntimeExceptionor declaredIOExceptionto-32603when a response is appropriate.doPostguard best-effort commits a zero-length HTTP 500 before rethrowing an escapedError, 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.application/json-rpcare set explicitly at normal JSON-RPC servlet exits; the customHttpStatusCodeProviderconfiguration and the now-unusedCachedBodyRequestWrapperare removed.-32600 "Invalid Request"withid: null. Non-null scalarparamsalso gets-32600; a valididis echoed, while a missing, null or invalididbecomesnull. In a batch only the offending element gets the error and the other elements still execute. An explicitid: nullon an otherwise valid request is not rejected by these checks; its final semantics, and whether to rejectparams: null, are decided with [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) tronprotocol/java-tron#6676.idand answers a valid notification with an empty 200 body.Why are these changes required?
An exception without an
@JsonRpcErrorsmapping currently produces (Java 8):{"jsonrpc":"2.0","id":1,"error":{"code":-32001,"message":null,"data":"java.lang.NullPointerException"}}message: nullviolates JSON-RPC 2.0 section 5.1 (on Java 17 it becomes a helpful-NPE string echoing internal class and method names),dataexposes internal types, and-32001is 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 asOutOfMemoryError,StackOverflowErrorandTronErrorwere 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. Scalarparamshad the same result after a registered method reached argument matching; an unknown method returned-32601before inspectingparams. In a batch, framework exceptions produced only-32603withid: nulland stopped further processing.-32603is the Internal error defined by the specification and matches Besu'sRpcErrorType.INTERNAL_ERRORclassification. Rejecting Boolean IDs is deliberately stricter than go-ethereum, following section 4 (String / Number / Null). Section 4.2 requires structuredparams; java-tron classifies a non-null scalar as-32600at the request-envelope layer, matching Besu's error-code classification, while geth classifies it as-32602during method-argument parsing. For this malformed shape without anid, java-tron returnsid: 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
paramschanges from-32601to-32600, while the same method with validparams: []remains-32601. A malformed request without anidis 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-32603for unmapped exceptions and a null method, bounded WARN/DEBUG logging by method and exception class, fatal error propagation includingTronError(direct, wrapped, cyclic cause chain).JsonRpcErrorSanitizationIntegrationTest(19 tests) - through a realJsonRpcServerandJsonRpcServlet: unmapped exceptions sanitized on the wire, four fatal categories escaping the server, servlet best-effort empty-500 handling, fixed messages forExecutionException/InterruptedException, chain identity keeping-32001with sanitized details, business messages preserved, batch isolation, transport contracts, scalar-paramsand request-ID rules, and real dispatch of supportedparamsshapes.JsonRpcDispatchContractTest(8 tests) - characterization of jsonrpc4j 1.6 dispatch (arity, null array elements, overload selection, scalarparams) so a later framework upgrade cannot change behavior silently.JsonRpcServletTest(39 tests) - request ID and scalar-paramsvalidation, exact pass-through of supported shapes, batch isolation, catch-all ID preservation, declaredIOExceptionhandling 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.CachedBodyRequestWrapperTestis 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, andgit diff --checkalso pass.Compatibility
Breaking, limited to observable failure-handling paths; no request that succeeds today starts failing.
-32001, exception message (may benull),data= class name-32603 "Internal error", nodatanet_version/eth_chainIdfailure-32001, underlying message,data= class name-32001 "Chain identity unavailable",data "{}"ExecutionException/InterruptedException-32000, causetoString()/null-32000 "Internal error"-32603/id: null, then processing stops-32600 "Invalid Request",id: null; batch siblings continueparams-32603/id: nulland stops. An unknown method returns-32601before checkingparams.-32600 "Invalid Request", nodata; valididpreserved, otherwiseid: null; batch siblings continue. Envelope validation precedes method lookup, so unknown + scalar changes to-32600, while unknown +params: []stays-32601.Error(VirtualMachineError/ThreadDeath/LinkageError/TronError)-32001/-32000handleRequestthrowsIOException-32603; a notification remains response-freeUnchanged: 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:
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 fromx-tron-error-modelindocs/api/openrpc.json.-32001behavior or on the previous chain-identitymessage/data, and that their error classification accounts for the new-32603responses.Follow up
jsonrpcandmethodmembers, whether to rejectparams: null, and the final semantics of an explicitid: nullon 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 inJsonRpcServletand theTronJsonRpcannotation blocks (see Extra details).JsonNodeID.-32000catch sites ineth_call/eth_estimateGas/buildTransaction.-32700and loses the request id.JsonRpcServlet.doPost, on a best-effort basis.Extra details
maxResponseSizecontinues 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
paramsand noidis malformed rather than a valid notification, so it receives-32600withid: null. Envelope validation also intentionally precedes method lookup; clients probing method availability should use a structurally validparamsarray or object.This PR overlaps the request-envelope validation planned in tronprotocol#6676 in
JsonRpcServletand theeth_getLogs@JsonRpcErrorsblock ofTronJsonRpc. 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:
handleRequestis required and why the chain-identity cause must be retainedCloses tronprotocol#6941
Refs tronprotocol#6676