NethermindEth / NethermindEth/juno
jsonrpc: panics in RPC handlers/validators crash the connection or vanish silently in batch requests
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 444
- Forks
- 244
- Avg merge
- 2d 15h
- Merged PRs (30d)
- 78
Description
Description
jsonrpc.Server.handleRequest does not recover from panics raised by RPC method handlers or by parameter validators (s.validator.Struct(...), called from buildArguments/validateParam). There's already a TODO in the code:
// TODO: add recover() to catch panics from handlers/validators and return a JSON-RPC internal error
// instead of crashing the HTTP connection
There is a problem with both a single request and a batch request.
- Single (non-batch) request: handleRequest runs directly in the HTTP/WebSocket connection's goroutine. An unrecovered panic propagates up to net/http's own recover and terminates that one connection. With logs but without valid JSON-RPC response is returned to the client.
- Batch request: each element is processed via s.pool.Go(...). handleBatchRequest never calls s.pool.Wait(), so a panic is silently swallowed: the panicking element's response is simply missing from the batch array, with no error and nothing in the logs
Acceptance criteria
- A panic raised inside an RPC method create JSON-RPC Internal error response instead of crashing the connection.
- A panic raised during validateParams is recovered the same way.
- For notification requests per the JSON-RPC 2.0 spec it should still be logged, just not answered.
- A panic in one element of a batch response must show up as an Internal error for that element, while the rest of the batch completes normally.
Documentation link(s)
- about notifications in JSON-RPC 2.0: https://www.jsonrpc.org/specification#notification
- about panic propagation in pool: https://pkg.go.dev/github.com/sourcegraph/conc/pool
- about panic in http: https://pkg.go.dev/net/http#pkg-overview:~:text=Request%2E-,If,ErrAbortHandler%2E
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at jsonrpc.Server.handleRequest and handleBatchRequest, then trace buildArguments and validateParam where validator panics can occur. Review how s.pool.Go and s.pool.Wait handle panics before running the existing JSON-RPC tests. Done means handler and validation panics produce Internal errors for requests and batch elements, while notification panics are logged without responses and other batch elements still complete.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100