Skip to content

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

Open
waynercheung wants to merge 1 commit into
tronprotocol:release_v4.8.3from
waynercheung:feat/jsonrpc-error-sanitization
Open

waynercheung wants to merge 1 commit into
tronprotocol:release_v4.8.3from
waynercheung:feat/jsonrpc-error-sanitization

Conversation

@waynercheung

@waynercheung waynercheung commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

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 are rethrown as the actual cause instead of being answered. A shared allocation-free, cycle-safe scan has no cause-depth cutoff; servlet dispatch catches use it before ordinary logging or error mapping as well.
  • net_version / eth_chainId keep their documented -32001 through an explicit mapping ("Chain identity unavailable", data "{}"); ethChainId() keeps the cause, rethrows a fatal cause before logging, and logs the first failure per exception type at WARN with repeats at DEBUG (deduplication, not a time-based limiter; a recurrence after recovery does not warn again).
  • ExecutionException / InterruptedException on the asynchronous log query get a fixed "Internal error" message; LogBlockQuery rethrows a fatal cause before its WARN (the executor wraps a task-thrown Error in ExecutionException), logs other causes 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 catch RuntimeException, IOException and Error, inspect the complete cause chain before logging, and rethrow a classified fatal cause. Other escaped failures become -32603 under the existing ID rules. This also covers non-fatal Errors from interceptor hooks outside jsonrpc4j's method-invocation catch; any partial dispatch output is discarded.
  • Recoverable batch failures in request serialization, dispatch or response parsing produce an element-specific -32603, retaining earlier results and continuing with later requests unless the existing response budget overflows. Each batch logs only its first escaped non-fatal dispatch failure at ERROR with the Throwable; subsequent failures use DEBUG with only the index and exception class. This state is request-local, and fatal inspection precedes logging. Malformed response bytes are discarded; only the replacement error is charged. The original pre-parse size check and strict > boundary remain: bytes exceeding the remaining budget get -32003 even if malformed.
  • An outer doPost guard best-effort commits a zero-length HTTP 500 before rethrowing an escaped Error. On this cleanup path only, it unwraps up to 16 ServletResponseWrapper layers because the production HttpInterceptor's response wrapper does not delegate flushBuffer. Self-reference, an unresolved/deeper chain or a non-HTTP inner response abandons cleanup without calling response methods through that wrapper. Cleanup failures never replace the original fatal. The client may receive an empty 500 or a closed connection; if cleanup cannot commit, container fallback remains possible. 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) #6676.
  • The single-request catch-all preserves a valid request id; when id is absent it keeps the existing empty 200, while explicit id: null gets an error with id: null. A recoverable batch failure without id still produces an error with id: null. Responses returned normally by jsonrpc4j are forwarded, including its existing error responses to some requests without id; the resolver's new error mapping still applies. Notification response-suppression rules are preserved, not normalized; normalization is deferred to [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) #6676.

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 #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. Successful notifications remain response-free; this change does not yet unify suppression of error responses across dispatch and servlet catches.

This PR has been tested by:

  • Unit Tests

  • Manual Testing

  • JsonRpcErrorResolverTest (17 tests) - mapped code / data priority, message defaults, sanitized unmapped exceptions, bounded logging, first-occurrence tracking per operation and exception type, and fatal propagation including null/ordinary/deep/cyclic cause chains and TronError.

  • JsonRpcErrorSanitizationIntegrationTest (29 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 asynchronous and chain-identity messages, business messages preserved, batch isolation, transport and request-envelope contracts. A real preHandleJson interceptor proves that non-fatal Errors escape the raw server and are recovered by the servlet; direct and Error-wrapped fatal causes still propagate. No-ID unknown-method, arity and unmapped-exception responses are characterized on both single and batch paths without changing suppression rules.

  • 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 (72 tests) - request validation and pass-through; all three batch recovery points; exact-limit, replacement-overflow and missed/double-accounting guards; per-batch logging including non-fatal Errors and reset across requests; fatal-after-failure stops dispatch without another log; missing versus explicit null ID on escaped failures; single/batch partial-output discard checked against the raw markers and with strict trailing-token rejection; direct and wrapped fatal propagation; response-wrapper cycles/depth limits and cleanup failures.

  • LogBlockQueryFailureTest (4 tests) - cause logging, interrupt-flag restoration, and fatal causes rethrown without the failure WARN, both from a mocked Future and through a real executor's FutureTask.

  • TronJsonRpcImplChainIdentityTest (6 tests) - first-occurrence WARN with the cause and DEBUG repeats without it, no reset after recovery, a key shared with net_version, and wrapped or direct fatal causes rethrown without logging or consuming the first WARN.

  • JsonRpcServletJettyTest (3 tests) - filtered and unfiltered fatal paths plus a healthy filtered request. For JSON, HTML and plain-text Accept values, the production-filter test requires server-side evidence of committed status 500 and zero content length; an empty 500 or a closed connection is then accepted. The observation filter never commits the response.

A package-private server setter supports servlet tests without changing production initialization. CachedBodyRequestWrapperTest is removed together with the class. On 2026-09-08, a cleanTest --no-build-cache JDK 17 (arm64) run passed 28 test classes / 330 tests (0 failures, 0 errors, 0 skipped); the seven focused classes above then contained 131 tests. checkstyleMain, checkstyleTest, and git diff --check also pass. Separately removing the single and batch Error catches made six distinct new tests fail for each mutation; both catches were restored before the full run. On 2026-09-15 the same suite and both Checkstyle tasks were re-run on the branch rebased onto release_v4.8.3 (0d19485318, which already carries the slf4j 2.0.17 / logback 1.3.16 / jackson 2.18.10 upgrade from #6950 and the HTTP error sanitization from #6954) with the same 28 classes / 330 tests passing; the logging assertions that read logback's Logger, ListAppender and ILoggingEvent hold under logback 1.3.16. On 2026-09-24, after the review changes to fatal ordering and chain-identity logging, the same cleanTest --no-build-cache suite passed 28 test classes / 338 tests (0 failures, 0 errors, 0 skipped) on the same base; the seven focused classes above now contain 139 tests, and both Checkstyle tasks and git diff --check pass. Moving the chain-identity fatal check after logging, removing the LogBlockQuery fatal check, logging under different wording or at TRACE before either fatal check, and making first-occurrence tracking always report a first occurrence each failed the intended new tests; all four were reverted before the final run. Java 8/x86 execution was not performed on this arm64 machine; CI covers it.

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
Single handleRequest throws non-fatal RuntimeException / IOException hidden by the old servlet-level entry point with a valid ID, HTTP 200 / -32603; without id, the existing empty body is retained
Non-fatal Error escapes handleRequest, e.g. from preHandleJson Single: caught by the old handle entry point without guaranteeing a complete JSON-RPC error response (empty HTTP 200 if no bytes have been written). Batch: escapes to outer/container handling. HTTP 200 / -32603 under the existing ID rules; partial output is discarded, batch siblings continue within the existing response budget. A no-ID single catch stays silent; explicit id: null and no-ID batch failures get an error with id: null. Classified fatal causes still propagate.
Recoverable batch request-serialization, dispatch or response-parsing failure Serialization failures, escaped dispatch RuntimeExceptions and response-parsing failures each return a single -32603 with id: null, discarding earlier results and skipping later elements. A dispatch IOException was not caught at all and propagated to outer handlers without this JSON-RPC response guarantee. preserve earlier results, append -32603 for the failed element and continue within the existing overflow rules; missing id still produces id: null

Unchanged by this PR: successful responses, HTTP 200 whenever a normal JSON-RPC response is produced, existing dispatch and method validation for missing / null / Array / Object params, code / data of the remaining existing error mappings, deliberate business messages such as "filter not found", gRPC and non-JSON-RPC HTTP APIs. The JSON-RPC code on release_v4.8.3 is identical to the develop baseline (4a21592f95), which has 62 mappings. Removing the uninstall lookup-miss mapping belongs to #6951; if that change lands first, this PR rebases onto a 61-mapping baseline and adds the two chain-identity mappings, leaving 63 combined. 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) #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) #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.
  • Notification normalization is deferred to [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) #6676 as discussed in [Feature] Standardize JSON-RPC error mapping and exception boundaries #6941. Characterization tests record current behavior: jsonrpc4j-generated unknown-method, arity and unmapped-exception errors without id are forwarded; a single servlet catch without id is silent, explicit id: null gets an error, and a batch failure without id emits id: null. Future normalization must consider forwarding and synthesized-error exits together; this PR does not preselect that policy.
  • Review the five mapped -32000 catch sites across three methods (eth_call, eth_estimateGas, buildTransaction) that still forward the underlying exception message; changing those public business-error messages needs its own compatibility review.
  • 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.

The per-batch log policy bounds only the servlet's escaped-dispatch failure log point, not business, resolver or container logging, and is not a cross-request rate limiter. Enabling DEBUG exposes repeated failure indices and types but not their Throwable or message. Single-request non-fatal dispatch failures are logged individually at ERROR with the Throwable.

The fatal cleanup bypasses response wrappers locally; it does not modify shared HTTP wrapper semantics. The missing flush delegation in CharResponseWrapper / ServletOutputStreamCopy can be discussed with #6936 independently. The embedded-Jetty regression installs the actual HttpInterceptor; an outer observer only records the underlying response's committed state, status and zero content length, and never commits it on behalf of the servlet.

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 #6676 in JsonRpcServlet and the eth_getLogs @JsonRpcErrors block of TronJsonRpc. There is no dependency between the two: this PR can land first and #6676 can build on the pre-dispatch checks added here; if #6676's PR lands first, this one will be rebased.

Fatal classification. VirtualMachineError, LinkageError, ThreadDeath and TronError, found anywhere on the cause chain, form the propagation set. This is a chosen policy, not a proof that every other Error is safe to recover from. Errors without a classified fatal cause follow the existing response-suppression and batch-recovery rules, using the sanitized -32603 where an internal-error response is emitted; that reports a failed call, not a healthy or recovered node. The policy narrows develop, where method-invocation Errors, including OutOfMemoryError, were answered as -32001 with the class name, and applies the same classification at the method-invocation and dispatch boundaries, as discussed in #6941.

Local safeguards and replacement conditions.

  • Unwrapping response wrappers on the fatal cleanup path: can be removed after shared flush delegation is fixed and production-filter tests pass without it.
  • The best-effort empty 500 before rethrowing a fatal Error: can be removed or replaced after container handling provides equivalent protection across the affected paths and formats, with the HTTP contract verified.
  • The Boolean/object/array id and scalar params checks: needed because handleRequest throws for these envelopes where handle() swallowed them; they will be integrated with [Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) #6676's validation, in coordination with that change, rather than maintained as duplicate predicates.

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 the first occurrence per exception type and repeats only at DEBUG, after fatal classification; each batch logs its first escaped dispatch failure at ERROR with a stack and repeats only at DEBUG without a stack/message; 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 #6941
Refs #6676

throw new JsonRpcInternalException(e.getMessage());
// Mapped errors bypass the resolver's unhandled-exception log, so record the complete
// cause once per failure episode at the lookup boundary.
if (chainIdentityLookupFailed.compareAndSet(false, true)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD] Simplify the chain-identity logging state before extending it

This adds a dedicated failure/recovery state machine for one log point, although response sanitization and fatal propagation do not require that state. It also updates chainIdentityLookupFailed and logs before the resolver can inspect the cause chain: a RuntimeException wrapping VirtualMachineError or TronError is therefore treated as a recoverable outage before the actual fatal cause is rethrown. Concurrent success and failure calls can additionally make recovered describe one successful call rather than a stable recovery episode.

Suggestion: first confirm that repeated chain-identity failures cause a real log-flood problem. If not, remove the dedicated state and keep a simple sanitized failure path. If suppression is required, prefer a shared rate-limited logging mechanism; at minimum, classify and rethrow wrapped fatal causes before any state transition or log, with a regression test for that boundary.

@waynercheung waynercheung Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, confirmed with injected wrapped fatal causes: ethChainId logged and changed state before the resolver rethrew the original cause. I have not identified a production trigger on that lookup path.

Addressed in cd012e1d74:

  • Fatal causes are now classified first and rethrown as the original instance, before any logging, first-occurrence tracking or wrapping.
  • The failure/recovery flag and the recovery INFO are removed. ethChainId now shares first-occurrence tracking with the resolver: the first occurrence per operation and exception type is logged at WARN with the Throwable, and repeats at DEBUG without the Throwable or message. Both chain-identity methods use one operation key. This is deduplication, not rate limiting; a later recurrence after recovery does not emit another WARN.
  • LogBlockQuery had the same ordering issue, since its executor Future wraps a task-thrown Error in ExecutionException; it now checks for fatal causes before its WARN. findFatalCause is public so that the filters package can use the same classification.

Tests cover: a wrapped or direct fatal cause rethrown as the same instance with no log event at any level; a fatal cause not consuming the first WARN (both calls share the outer exception type); first-occurrence WARN and DEBUG repeats, across a recovery on the real null-block path and shared with net_version; and, for LogBlockQuery, a fatal cause both from a mocked Future and through a real executor's FutureTask.

// JsonRpcServer.handle catches Throwable and would swallow fatal errors rethrown by the
// resolver. Use the lower-level entry point so single and batch requests share a boundary.
rpcServer.handleRequest(new ByteArrayInputStream(body), bufferedResp.getOutputStream());
} catch (RuntimeException | IOException | Error e) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[QUESTION] Should unclassified Error types be recoverable?

This catch handles every Error, while rethrowIfFatal propagates only VirtualMachineError, ThreadDeath, LinkageError, and TronError. Every other subtype, including AssertionError and potentially library or JVM invariant failures, is converted into a normal -32603 response. The interceptor AssertionError case is deliberate and tested, but that does not establish that every non-allowlisted Error is safe to continue from.

Suggestion: consider propagating Error by default and explicitly allow only the narrow recoverable subtype(s) required here, or document why the current fatal allowlist is complete for this boundary.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed: these four categories are a propagation policy, not proof that every other Error is safe to recover from.

The compatibility reason is that method-invocation Errors already enter jsonrpc4j's error-response path (on develop this includes OutOfMemoryError, answered as -32001 with the class name). As @lxcmyf suggested in #6941 (#6941 (comment)), this PR applies the same classification to Errors escaping dispatch. Changing only the servlet would recreate the method/interceptor inconsistency; changing both would alter method-level responses and batch continuation.

I propose retaining this policy here, and I have documented the trade-off in the PR description under "Fatal classification". Errors without a classified fatal cause follow the existing response-suppression and batch-recovery rules, using a sanitized -32603 where an internal-error response is emitted. This reports a failed call, not a healthy or recovered node.

@bladehan1, does keeping this policy with the trade-off documented address your concern? @lxcmyf, since it follows your #6941 suggestion, please weigh in if you see it differently. A stricter default would need a coordinated change across the resolver, the dispatch catches, the cause-chain policy and the compatibility tests, so I would prefer to keep it out of this PR unless you both think it belongs here.

try {
rpcServer.handle(cachedReq, bufferedResp);
} catch (RuntimeException e) {
// JsonRpcServer.handle catches Throwable and would swallow fatal errors rethrown by the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[DISCUSS] Keep independent protocol and HTTP changes separable

Using the lower-level jsonrpc4j entry point is necessary for fatal propagation, but this PR also combines per-element batch recovery, partial request-envelope validation that overlaps #6676, and JSON-RPC-local bare-500/wrapper-unwrapping behavior. These concerns have different ownership and rollback paths; keeping them together makes a future jsonrpc4j or Jetty change harder to evaluate against the core exception fix.

Suggestion: keep error sanitization, fatal propagation, and batch recovery here; move complete envelope semantics to #6676 and handle default-error-page/wrapper behavior in an HTTP/Jetty change. If they must remain together, document these boundaries and the conditions for removing the local workarounds.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I propose keeping these safeguards here, with explicit replacement conditions.

The ID and scalar-params checks prevent framework failures exposed by the handleRequest switch from being misclassified as internal errors. Complete envelope and notification semantics remain with #6676.

The best-effort empty-500 guard prevents an escaped Error from reaching Jetty's detail-bearing default response when cleanup succeeds. Unwrapping is needed because the production response wrapper does not delegate flushBuffer reliably. These protections cannot simply be removed without a replacement.

I have added the replacement conditions to the PR description under "Local safeguards and replacement conditions":

@waynercheung
waynercheung force-pushed the feat/jsonrpc-error-sanitization branch from 6ecb02c to cd012e1 Compare September 24, 2026 13:39
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. Scan complete cause chains without
allocating a visited set, detecting cycles without missing fatal causes.
Classify Throwables escaping dispatch before logging or mapping them.

Before rethrowing an Error, unwrap response decorators with a bounded
walk and make a best-effort attempt to commit an empty HTTP 500 on the
underlying response. Preserve the original error if cleanup fails, and
abandon cleanup without delegating into cyclic or unresolved wrappers.

Route single requests through handleRequest because the servlet handle
API catches Throwable. Catch RuntimeException, IOException, and Error at
both dispatch boundaries, then propagate a classified fatal cause or
return -32603 when a response is appropriate. This also covers non-fatal
Errors from interceptor hooks. Discard partial dispatch output on
failure.

Recover each failed batch element without discarding earlier results or
skipping later requests. Share internal-error construction across
serialization, dispatch, and response-parsing failures. Count only the
replacement when parsing fails, retaining existing overflow rules and
the current single/batch handling of requests without an id.

Bound unmapped-exception and chain-identity failure logging to the first
occurrence per operation and exception type. Classify fatal causes
before logging chain-identity and asynchronous log-query failures. Log
the first escaped dispatch failure in each batch at ERROR with its
cause; repeats use DEBUG with only the index and exception type.
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.
Exercise the real HTTP interceptor chain and observe commitment without
performing it on the servlet's behalf.

Test interceptor Errors with a real server and preserve the existing
notification response-suppression rules with characterization tests.

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 cd012e1 to 2d2d9df Compare October 2, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants