Skip to content

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

Description

@waynercheung

Summary

For an exception without an @JsonRpcErrors mapping, JSON-RPC falls back to jsonrpc4j's -32001 and returns the Java exception class name and the raw exception message to the client, for example:

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

This issue standardizes JSON-RPC error mapping and the exception boundaries:

  • Unmapped non-fatal exceptions return -32603 "Internal error"; Java exception class names and raw messages are no longer echoed.
  • Four fatal Error categories, including java-tron's TronError, propagate instead of being disguised as JSON-RPC error responses. That classification governs the resolver and the two dispatch boundaries; the outer doPost guard is a last-resort cleanup that covers any Error raised outside the conversion path, so it is not a guarantee that only these four categories can escape every stage of the servlet. Before rethrowing, the servlet makes a best-effort attempt to commit an empty HTTP 500 so the container does not render exception details.
  • An invalid request ID type or non-null scalar params returns -32600 "Invalid Request" instead of being swallowed on the single-request path or aborting the whole batch.

Only failure-path responses change. Whenever a normal JSON-RPC response is produced, its HTTP status remains 200; before propagating a fatal Error, the servlet best-effort commits an empty HTTP 500, while a failed attempt may leave a closed connection. Successful responses, gRPC and non-JSON-RPC HTTP API behavior remain unchanged.

This issue only covers the framework-level fallback and exception boundaries; it does not change how any method validates its own parameters. The null parameter of eth_getLogs in the example is a separate problem; even once it gets a null check, any other unmapped exception still takes this path.

Problem

Motivation

  • message: null violates JSON-RPC 2.0 section 5.1, which defines message as "A String providing a short description of the error" (null is not a String); on Java 17 it becomes a diagnostic string containing internal method signatures; data echoes the Java exception class name. None of these should be depended on by clients.
  • -32001 is registered in the public error catalog as a server-side internal error, yet the fallback files every unmapped exception there, including client input errors, so clients cannot tell them apart.
  • OutOfMemoryError / StackOverflowError are converted into ordinary error responses, masking an unrecoverable process state.
  • A single request with an invalid request ID gets HTTP 200 with an empty body. Scalar params has the same result after a registered method reaches argument matching; an unknown method is rejected earlier as -32601. In a batch, the servlet catch-all returns only a -32603 / id: null response for the framework exception and stops early, discarding prior results and skipping later elements.

Current State

  • Unmapped exceptions: the resolver returns null and jsonrpc4j falls back to ERROR_NOT_HANDLED (-32001). net_version / eth_chainId declare JsonRpcInternalException without an annotation and take exactly this path.
  • The asynchronous query in eth_getLogs / eth_getFilterLogs: ExecutionException leaks the cause's type through message; InterruptedException yields message: null and leaves the interrupt flag cleared.
  • Fatal errors: jsonrpc4j catches Throwable both at the method invocation layer and in handle(...).
  • Protocol level: a Boolean / object / array request ID throws IllegalArgumentException. Scalar params also throws after a registered method reaches argument matching; an unknown method returns -32601 before that check. Single-request handle(...) swallows the exception, while the batch servlet catch-all returns -32603 and stops early.
  • Unmapped exceptions at the method invocation stage leave no trace on the node (setShouldLogInvocationErrors(false)).

develop @ 4a21592 and GreatVoyage-v4.8.2.1 are both affected; verified on Java 8 and Java 17. Reproduce:

# unmapped exception (any unmapped exception gives the same shape)
curl -s -X POST http://127.0.0.1:8545/jsonrpc -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","method":"eth_getLogs","params":[null],"id":1}'
# invalid request ID type -> HTTP 200 with an empty body
curl -i -s -X POST http://127.0.0.1:8545/jsonrpc -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","method":"web3_clientVersion","params":[],"id":true}'
# scalar params -> HTTP 200 with an empty body
curl -i -s -X POST http://127.0.0.1:8545/jsonrpc -H 'Content-Type: application/json' \
  -d '{"jsonrpc":"2.0","method":"web3_clientVersion","params":5,"id":2}'

Limitations and Risks

  • Clients matching the old message / data will observe a change; the net_version / eth_chainId responses are registered in the public error catalog and need a synchronized update.
  • When a fatal Error propagates, the client receives a best-effort empty HTTP 500 or a closed connection; the current batch loses accumulated results. This is an intentional boundary.
  • Consensus, chain state and funds are not involved.

Proposed Solution

Proposed Design

Case HTTP status JSON-RPC code message data / notes
Unmapped non-fatal exception 200 -32603 "Internal error" no data; first (method, exception type) occurrence is WARN with the stack, repeats are DEBUG without stack/message
net_version / eth_chainId failure 200 -32001 "Chain identity unavailable" "{}" (explicit mapping; keeps the code registered in the public catalog)
ExecutionException / InterruptedException 200 -32000 "Internal error" "{}"; InterruptedException restores the interrupt flag
Non-fatal Error escaping dispatch (for example an AssertionError from a JsonRpcInterceptor hook, which jsonrpc4j does not catch) 200 -32603 "Internal error" no data; the fatal cause is classified before logging or response generation; the same ID rules apply, a single request without an id keeps its empty body, and in a batch only that element is affected
Fatal Error (VirtualMachineError / ThreadDeath / LinkageError / TronError, including wrapped ones) no JSON-RPC response N/A N/A servlet best-effort commits an empty HTTP 500 and rethrows the same Error; resource exhaustion may close the connection instead; propagation does not itself terminate the process
handleRequest throws IOException 200 -32603 when an ID is present "Internal error" no data; a no-ID single request remains response-free
Boolean / object / array request ID 200 -32600 "Invalid Request" id: null (2.0 section 4); in a batch only that element is affected
Non-null scalar params (single request or batch element) 200 -32600 "Invalid Request" no data; echo a valid id, otherwise use id: null; only the offending batch element is affected
Mapped errors 200 unchanged deliberate business messages such as "filter not found" unchanged "{}" unchanged

-32603 is the Internal error defined by JSON-RPC 2.0; its error-code classification matches Besu's RpcErrorType.INTERNAL_ERROR. Rejecting Boolean IDs is stricter than go-ethereum, following 2.0 section 4 (an ID is a String, Number or Null). Section 4.2 requires params to be structured. java-tron classifies non-null scalar params at the request-envelope layer as -32600, matching Besu's error-code classification (its HTTP status handling differs); geth classifies the same shape as -32602 at method-argument parsing. This is a difference in layering and error classification, not a claim that geth violates the specification.

Request-envelope validation takes precedence over method lookup: an unknown method with scalar params changes from -32601 to -32600, while the same unknown method with valid params: [] remains -32601. An object with scalar params and no id is not a valid Notification, because a Notification must first be a valid Request Object under sections 4 and 4.1; it therefore receives -32600 with id: null. This issue does not unify notification handling: errors returned by jsonrpc4j are forwarded, a single-request servlet catch without an id stays silent, and a recoverable batch failure without an id produces an error with id: null. Normalizing those into one rule is proposed for #6676, subject to confirmation there.

Key Changes

  • resolver: unmapped non-fatal exceptions become -32603; logging is bounded by (method, exception type), with the first occurrence at WARN carrying the Throwable and repeats at DEBUG without the Throwable/message; message precedence is annotation > exception > per-code default; walk the cause chain for four fatal Error categories and rethrow the actual cause. The scan allocates nothing, because a fatal cause may itself be an OutOfMemoryError, detects cycles with two pointers rather than a visited set, and applies no depth cutoff.
  • mapping annotations: add an explicit -32001 mapping for net_version / eth_chainId; give ExecutionException / InterruptedException a fixed message; log the cause and restore the interrupt flag at Future.get().
  • servlet: recoverable batch failures in request serialization, dispatch and response parsing produce an element-scoped -32603, keep earlier results and continue with later elements within the existing response-size rules; only the replacement error is charged when a malformed response is replaced. Each batch logs its first escaped non-fatal dispatch failure at ERROR with the Throwable and later ones at DEBUG with only the index and exception class, with request-local state. Dispatch single requests through handleRequest(InputStream, OutputStream) (handle(...) swallows fatal errors); catch RuntimeException, IOException and Error at both dispatch boundaries, rethrowing a classified fatal cause before logging and otherwise using the existing sanitized error path; separately, the outer guard best-effort commits an empty 500 before rethrowing an escaped Error, without allowing cleanup failures to replace it; validate request-ID and non-null params container types before dispatch and isolate batch elements.
  • The change is limited to the JSON-RPC layer of the framework module: JsonRpcErrorResolver, JsonRpcServlet, TronJsonRpc, TronJsonRpcImpl, LogBlockQuery.

Impact

  • Security: error responses no longer return Java exception class names or unaudited exception messages; propagating fatal errors stops masking an existing process-level failure signal.
  • Stability: unmapped exceptions get a stable fallback; a future method that misses a null check will not echo internal types.
  • Performance: the normal path is unaffected; full WARN stacks for unmapped exceptions are limited to the first occurrence of each method/type pair.
  • Developer Experience: errors become interpretable against the specification; unmapped exceptions at the method invocation stage start appearing in the node log.

Compatibility

Item Result
Breaking Change Yes, limited to failure and exception handling paths. Unmapped exceptions -32001 + class name -> -32603; net_version / eth_chainId keep the code but get a fixed message / data; ExecutionException / InterruptedException get a fixed message; invalid IDs, and scalar params after a registered method is selected, go from an empty body to an error response for single requests and from one -32603 plus early batch termination to an isolated -32600 for batch elements; an unknown method with scalar params changes from -32601 to -32600, while valid params: [] still returns -32601; four fatal categories go from an error response to an empty HTTP 500 or closed connection. Difference from geth: geth returns -32602 for scalar params and sends no response when such a malformed request has no id, whereas java-tron returns -32600 with id: null. Must be included in the release notes.
Default Behavior Change Yes. Only failure responses and request ID / non-null params container validation change; successful responses remain unchanged.
Migration Required Conditional. Clients matching the old message / data need to adjust.

The following remain unchanged: 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 62 existing mappings (4 asynchronous-exception mappings only gain a message; 2 chain identity mappings are added), gRPC and non-JSON-RPC HTTP API behavior.

Before merge: update the public error catalog (docs/api/openrpc.json in documentation-en; four entries: JSON_RPC_UNDERLYING_INTERNAL_ERROR, JSON_RPC_SERVLET_INTERNAL_ERROR, JSON_RPC_EXECUTION_ERROR, JSON_RPC_INTERRUPTED); check whether gateways / SDKs / monitoring depend on the old -32001 behavior.

The comparisons above are against develop. For a non-fatal Error escaping dispatch, the old single-request JsonRpcServer.handle catches and logs the Throwable without guaranteeing a complete JSON-RPC error response: a hook that fails before output produces an empty HTTP 200, while a later failure may leave partial output. A batch escapes to outer/container handling. The new dispatch catches return -32603 under the existing ID rules, discard partial output, and recover per batch element within the existing response budget. The best-effort empty HTTP 500 remains the intended handling for the four fatal categories; it was only an intermediate branch behavior for non-fatal dispatch Errors.

Acceptance Criteria

  • code / message / data for every row of the table are pinned by tests against a real JsonRpcServer / JsonRpcServlet.
  • All four fatal categories (including wrapped causes) escape the servlet; the best-effort empty 500 never echoes the fatal marker, and cleanup failure never replaces the original Error.
  • Escaped RuntimeException / IOException follows the documented ID/notification behavior.
  • A non-fatal Error escaping dispatch is classified before logging, then answered with -32603 under the same ID and batch rules; a classified fatal cause at the same boundary is still rethrown unchanged.
  • Resolver logging emits one WARN per method/type pair; ethChainId() logs failure/recovery transitions; Future.get() logs the cause and restores the interrupt flag.
  • code / data of the 62 existing mappings are unchanged.
  • Whenever a normal JSON-RPC response is produced, HTTP 200, content type and the batch and response size limits are unchanged; the three notification shapes above keep their current behavior and are pinned by characterization tests.
  • Non-null scalar params returns -32600 "Invalid Request" for single and batch requests; a valid id is preserved and other batch elements continue.
  • Request-envelope validation precedes method lookup: unknown method + scalar params returns -32600, while the same unknown method + params: [] remains -32601.
  • A characterization test pins jsonrpc4j dispatch behavior so a later upgrade cannot drift silently.

Follow-up

Outside the scope of this issue and not blocking its closure:

Additional Notes

Activity

  1. waynercheung commented on Sep 2, 2026

    @waynercheung
    CollaboratorAuthor

    Updated the proposal body after review; the scope is unchanged. What changed:

    • The fatal-error boundary now lists java-tron's TronError alongside the three JVM categories, and states what the servlet does before rethrowing: a best-effort attempt to commit an empty HTTP 500 so the container does not render exception details. A client may instead observe a closed connection; propagation does not itself terminate the process.
    • Unmapped-exception logging is bounded per (method, exception type): the first occurrence is logged at WARN with the stack, repeats at DEBUG without it. Chain identity lookup failures are logged on state transitions only.
    • Added the row for an IOException escaping handleRequest (-32603, same ID rules) and the matching acceptance items.
  2. halibobo1205 commented on Sep 4, 2026

    @halibobo1205
    Collaborator

    Thanks for the detailed proposal. The overall direction looks good. A few edge cases may be worth clarifying or covering in tests:

    1. Interaction with the production response wrapper

      FullNodeJsonRpcHttpService installs HttpInterceptor, which passes a CharResponseWrapper to the servlet. Its flushBuffer() may not commit the underlying response when no wrapper-local writer or stream has been created. In a Jetty test with the production filter enabled, the intended empty HTTP 500 resulted in a non-empty default error response. A Jetty + HttpInterceptor integration test could help confirm the expected behavior.

    2. Per-element batch handling

      If handleRequest() throws RuntimeException or IOException, the batch path should preserve completed results and continue processing subsequent elements where possible. A [success, throws, success] test would be useful.

    3. Wrapped fatal errors

      It may be safer to apply the same cause-chain classification at the servlet catch boundaries as well as in the resolver, so both paths follow the same exception policy.

    4. Notifications

      It would be helpful to verify that notifications remain response-free when method resolution or invocation produces an error.

    These look like implementation and test-coverage details rather than changes to the main proposal.

  3. waynercheung commented on Sep 4, 2026

    @waynercheung
    CollaboratorAuthor

    @halibobo1205 Thanks, these are good catches. I checked all four against the current branch. Three are implementation and test gaps; the fourth requires an explicit compatibility decision, because the spec-compliant behavior changes existing wire responses.

    1. Production response wrapper. Confirmed. The embedded-Jetty test registers the servlet directly and never installs HttpInterceptor, and the servlet unit test uses a mock response whose flushBuffer() does commit, so both missed this. In the observed production filter path the guard does not commit the underlying response: CharResponseWrapper.flushBuffer() does not call super.flushBuffer(), and ServletOutputStreamCopy does not override flush(), so neither wrapper path reliably propagates a flush downwards. The Error then arrives at Jetty with an uncommitted response and the default error page is rendered.

    I will add an integration test that wires the production filter chain, and have the guard resolve the underlying response through ServletResponseWrapper.getResponse() before committing, so the change stays inside JSON-RPC. The unwrapping will be used only on the fatal cleanup path and will handle nested wrappers.

    On the guarantee: when the bare response commits successfully, the client observes an empty HTTP 500 with no exception details. If the cleanup itself fails under a fatal condition, the result stays best effort and the client may see a closed connection or the container's own handling. I will state it that way rather than promising that details can never be rendered.

    Neither wrapper path reliably propagates a flush for any caller, not only this one. Rather than widen this change, I would like to raise that on #6936 first and let you decide whether it belongs there or in a separate issue.

    2. Per-element batch handling. Agreed. The proposal records losing the accumulated batch only as the fatal Error boundary, so recoverable failures were meant to stay element-scoped, but the implementation does not do that: a RuntimeException or IOException from one element discards the accumulated results and returns a single-element array. Two sibling paths do the same and will change with it: sub-request serialization failure and sub-response parse failure. All three will append an element-scoped -32603, preserve completed responses, and continue, reusing the existing response-size accounting. Fatal Errors still abort the batch.

    Tests: [success, RuntimeException, success] and [success, IOException, success], since the change covers both; a malformed sub-response followed by a successful element; a wrapped fatal aborting both the single and batch paths.

    3. Wrapped fatal errors. Agreed. I will centralize the cycle-safe cause-chain classification and apply it before the servlet catch paths translate RuntimeException or IOException into -32603, so a wrapped fatal Error is treated consistently whether it reaches the resolver or either handleRequest catch boundary.

    4. Notifications. Agreed on the target, with a correction to the baseline and one classification question for you.

    Today a request without id is response-free only when the call succeeds. Measured with jsonrpc4j 1.6 and the resolver currently on develop:

    request without id today on develop after this issue, if nothing else changes
    known method, succeeds empty empty
    unknown method -32601 -32601
    argument count mismatch -32602 -32602
    method throws, unmapped -32001, raw message and exception class name -32603

    The method throws, unmapped row matters for this discussion: today a client that asked for no response still receives the exception message and class name. So making notifications response-free on failure paths is a change rather than a preservation, and I will list it under breaking changes with these rows.

    The classification question: deciding what to suppress requires deciding what counts as a notification, and this issue deliberately does not validate jsonrpc or method. Suppressing every response for an object without an id would swallow genuinely malformed requests. Measured on the same setup, all of these return -32601 today and have no id:

    {"jsonrpc":"2.0"}                 no method
    {"jsonrpc":"2.0","method":5}      method is not a String
    {}                                empty object
    {"method":"nope"}                 no jsonrpc member
    

    I see two ways forward and would rather you pick:

    Suppression under the first option applies to whatever the dispatcher produced, so the rows above are representative rather than exhaustive: mapped method errors and parameter-conversion failures for otherwise valid notifications follow the same rule.

    Either way, a malformed object without an id, for example one carrying scalar params, stays an Invalid Request and receives -32600 with id: null. If we take the first option, a valid notification must not receive an appended error node in a batch either.

    I will update the acceptance criteria and the PR tests accordingly. Thanks in particular for exercising the production filter path.

  4. waynercheung commented on Sep 7, 2026

    @waynercheung
    CollaboratorAuthor

    @halibobo1205 Implementation status for the four points above.

    Items 1 to 3 are implemented and verified. The latest JDK 17 arm64 run passed 312 tests across 28 classes, with no failures, errors or skipped tests. Main and test Checkstyle also passed.

    • The fatal-cleanup path resolves the underlying response before making a best-effort attempt to commit an empty HTTP 500, because the production filter's response wrapper does not delegate flushBuffer. The walk is bounded and allocation-free, and a self-reference, an unresolved chain or a non-HTTP inner response abandons cleanup rather than calling response methods through that wrapper. A regression test installs the real HttpInterceptor and requires server-side evidence that the underlying response was committed with status 500 and zero content length; the observing filter never commits it on the servlet's behalf.
    • Recoverable batch failures in request serialization, dispatch and response parsing are handled per element, preserving earlier results and continuing within the existing response-size limits. When a malformed response is replaced, only the replacement bytes are charged; the pre-parse size check still takes precedence.
    • The cause-chain fatal classification is shared by the resolver and both dispatch catches. It allocates nothing, because a fatal cause may itself be an OutOfMemoryError, and it has no depth cutoff, so a fatal cause deep in a chain is still found while cyclic chains still terminate.

    Two notes that came out of implementing this.

    • Continuing after a batch element fails would have multiplied the ERROR-with-stack log point by the batch size. Each batch now logs only its first escaped non-fatal dispatch failure at ERROR with the Throwable; later failures use DEBUG carrying only the index and exception class. That state is request-local, and the fatal check still precedes logging.
    • The pre-change baseline for batch failures is finer than I described. On develop the dispatch call is wrapped only in catch (RuntimeException), so a dispatch IOException propagated to outer handlers instead of producing a JSON-RPC error, while the three caught paths returned a single -32603 with id: null regardless of the element's own id. I will state this split in the compatibility section above.

    Notification normalization (item 4) has not been implemented. The current behavior remains: errors returned by jsonrpc4j are forwarded, a single-request servlet catch without an id is silent, and a recoverable batch failure without an id produces an error with id: null.

    Would you prefer the narrow notification classifier in this issue, or defer normalization to #6676? I will update the implementation notes for items 1 to 3 separately, and finalize the notification-related compatibility statement and acceptance criteria once we agree on that choice.

  5. lxcmyf commented on Sep 7, 2026

    @lxcmyf
    Collaborator

    For notification handling, I would defer normalization to #6676. A narrow classifier here creates a second, temporary definition of a valid notification while the semantics of jsonrpc/method, params: null, and id: null are still undecided. That makes behavior dependent on landing order. Keeping characterization tests in this PR, then reusing one canonical envelope/notification predicate from #6676 across both forwarded and synthesized errors, seems safer.

    One boundary question: the proposal says only VirtualMachineError, ThreadDeath, LinkageError, and TronError propagate, but the current doPost guard catches and rethrows every Error. handleSingle and executeBatchRequest only catch RuntimeException | IOException, so an AssertionError from preHandleJson (outside jsonrpc4j invocation catch) or another non-classified Error would bypass the resolver and become an empty HTTP 500. Is that intentional? If not, could we add single/batch tests for a non-fatal Error at that boundary and translate it to -32603 per element, while rethrowing only when findFatalCause returns a match? Otherwise the compatibility section should explicitly state that all Error subclasses escaping dispatch are propagated.

  6. waynercheung commented on Sep 8, 2026

    @waynercheung
    CollaboratorAuthor

    @lxcmyf Both points accepted. The second one is a real gap.

    Notification. Deferring to #6676 is the better call, for the reason you give: a narrow classifier here would be a second definition of a valid envelope while jsonrpc, method, params: null and id: null are still open there, and it would make the result depend on landing order. This issue will preserve the existing response-suppression rules and pin them with characterization tests; the error-mapping and recovery changes still apply under those rules. I will also drop the blanket "valid notifications remain response-free" wording, because three shapes still differ: errors returned by jsonrpc4j are forwarded, a single-request servlet catch without an id is silent, and a recoverable batch failure without an id produces an error with id: null. Deferring means recording those, not claiming they are already unified. @halibobo1205, you raised this item, so say if you would rather have it here. @0xbigapple, does taking that normalization into #6676 fit the scope you planned?

    The Error boundary. Not intentional. I checked the jsonrpc4j 1.6 bytecode:

    • JsonRpcBasicServer.handleRequest(InputStream, OutputStream) has an exception table covering only JsonParseException and JsonMappingException, and preHandleJson is invoked inside that range, so an Error raised there leaves jsonrpc4j uncaught and also escapes both servlet dispatch catches.
    • An Error raised during method invocation is caught by handleObject's catch (Throwable) and reaches the resolver, where findFatalCause returns null for something like an AssertionError, so the response is -32603.

    The same AssertionError therefore produces -32603 from one layer and reaches the best-effort empty-500 guard from the other, and the existing AssertionError test only covers the method-invocation path. A RuntimeException from the same hook is already mapped to -32603, so the inconsistency is specific to Error subclasses. This is an exception-handling boundary that is inconsistent with the stated policy; MetricInterceptor.preHandleJson is empty in production, so I am not claiming a client-triggerable failure.

    Planned fix, in the direction you suggest:

    • Extend both dispatch catches to include Error, and apply findFatalCause before any logging or response generation. A matching fatal cause is rethrown unchanged; anything else uses the existing sanitized -32603 path with the current ID policy and per-element batch recovery. The helper and the per-batch log record accept Throwable accordingly.
    • The outer doPost guard stays a last-resort, best-effort cleanup. It also covers errors raised outside the conversion path, in body reading, envelope parsing and response writing, where there may be no usable request ID and no safe way to still generate an error response, so it should not be turned into a general JSON-RPC translator.
    • The proposal will scope the four fatal categories to the resolver and the dispatch boundaries, and describe the outer guard separately, rather than implying that only those four categories can escape every stage of the servlet.
    • Tests through a real server, injecting the failure from preHandleJson: a single request with an ID maps to HTTP 200 with -32603, the ID preserved and no internal detail; [success, AssertionError, success] keeps three results in order; a direct and a wrapped fatal at the same boundary are rethrown unchanged and stop later batch elements.

    For the compatibility table I will keep comparing against develop, where JsonRpcServer.handle swallows this Throwable and logs it, so a single request currently ends as HTTP 200 with an empty body rather than a 500. For this non-fatal dispatch Error, the empty-500 behavior is an intermediate state of the branch under development, not a change this issue makes relative to develop. The best-effort empty 500 remains the intended handling for the four fatal categories.

  7. halibobo1205 commented on Sep 8, 2026

    @halibobo1205
    Collaborator

    @waynercheung Thanks for addressing items 1–3 and for clarifying the existing notification behavior.

    I would prefer to defer notification normalization to #6676. Notification classification is closely tied to the complete envelope-validation rules for jsonrpc, method, params, and id, so handling it there should avoid maintaining a second, narrower definition in this issue.

    For #6941, it would be enough to document that error responses for notifications remain unchanged for now, and qualify any acceptance criterion that currently implies all valid notifications are response-free. #6676 can then implement and test notification suppression consistently across all dispatch and validation paths.

  8. waynercheung commented on Sep 8, 2026

    @waynercheung
    CollaboratorAuthor

    @halibobo1205 Thanks, that aligns with @lxcmyf's suggestion. Notification normalization will stay outside #6941.

    The proposal and the acceptance criteria have already been updated to record the existing paths rather than imply they are unified: errors returned by jsonrpc4j are forwarded, a single-request servlet catch without an id stays silent, and a recoverable batch failure without an id produces an error with id: null.

    One distinction worth being explicit about: what this issue preserves is the response-suppression rule, not the old error payloads. An unmapped invocation failure on a request without an id still changes from -32001 with the exception class name to the sanitized -32603. So notifications are not untouched by this issue; only the decision of whether a response is produced is left alone.

    I will make sure the PR's characterization tests cover each of those shapes before merge, including the ones that are not yet pinned: an unknown method, an argument count mismatch and an unmapped exception on a request without an id, and the servlet catch with an explicit id: null, which is a different branch from a missing id. That gives #6676 an enumerated starting point without adding a classifier here.

    @0xbigapple, notification normalization remains proposed follow-up work for #6676, subject to confirmation there. It is not a prerequisite for #6941.

    The separate non-fatal Error dispatch-boundary fix is still pending implementation and verification, as recorded in the proposal.

  9. waynercheung commented on Sep 22, 2026

    @waynercheung
    CollaboratorAuthor

    @lxcmyf @halibobo1205 The PR is #6985. The two outstanding implementation and test items from this thread are now addressed in it.

    Non-fatal Error at the dispatch boundary. Both dispatch catches now take RuntimeException | IOException | Error, with findFatalCause applied before any logging or response generation; a fatal cause is rethrown unchanged, anything else follows the sanitized -32603 path with the existing ID policy and per-element batch recovery. The outer doPost guard is unchanged as a last-resort cleanup. Through a real JsonRpcServer: testPreHandleJsonAssertionErrorEscapesRealServer pins the gap itself (an AssertionError from preHandleJson leaves jsonrpc4j uncaught), testServletSingleRecoversInterceptorAssertionError and testServletBatchRecoversInterceptorAssertionError show the servlet now answering -32603 with the ID preserved and [ok, error, ok] kept in order, and testNonFatalErrorIsSanitizedByRealServer covers the method-invocation layer, so both layers agree.

    Notification characterization. The shapes that were not yet pinned are now: testServletForwardsUnknownMethodNotificationError, testServletForwardsArityNotificationError and testServletForwardsUnmappedNotificationError for forwarded jsonrpc4j errors without an id, plus singleAssertionErrorWithoutId_keepsEmptyResponse, singleAssertionErrorWithNullId_keepsErrorResponse and batchAssertionErrorWithoutId_keepsErrorNodeAndContinues for the servlet catches, where a missing id and an explicit id: null are separate branches. No classifier was added; #6676 has been asked to confirm taking normalization (#6676 (comment)).

    The remaining documentation and compatibility checks are listed in the PR: updating the four public error-catalog entries in documentation-en, checking gateway / SDK / monitoring dependencies on the old catch-all -32001 and on the previous chain-identity message / data, and ensuring their error classification handles the new -32603 responses.

  10. removed this from the GreatVoyage-v4.8.3 milestone on Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    • Status
      No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions