Feat(rpc)/websocket gate

[NazariiDenha]

PR Type

Enhancement, Tests

---

Description

• Gate websocket RPC requests with shared limit

• Reject busy websocket calls without queueing

• Add ServerBusy JSON-RPC error code

• Test gate and websocket rejection behavior

---

File Walkthrough

	Relevant files
Documentation	1 files juno.go Update RPC gate flag usage text                                                    +4/-3
Enhancement	5 files gate.go Add non-blocking TryAcquire to gate                                            +18/-1    websocket.go Apply gate to websocket request handling                                  +54/-9    http.go Pass shared gate into RPC services                                              +4/-14    metrics.go Rename gate metrics to RPC gate                                                    +8/-7      node.go Create shared RPC gate in node                                                      +9/-2
Tests	2 files gate_test.go Add tests for TryAcquire behavior                                                +23/-0    websocket_test.go Test websocket gate busy rejection                                              +56/-0
Error handling	1 files server.go Add ServerBusy JSON-RPC error code                                              +3/-0

---

[github-actions]

Claude finished @NazariiDenha's task in 6m 23s —— View job

---

Review: PR #4014 (Feat/ws rpc gate)

Summary: This PR extends the existing HTTP admission-control gate (jsonrpc.Gate) to WebSocket RPC requests, using a new non-blocking TryAcquire() instead of the blocking Acquire() used by HTTP — so a busy server immediately replies with a server busy (-32004) JSON-RPC error on a WS connection instead of queuing (which would block that connection's message loop). The gate is now created once in node/node.go and shared between the HTTP and WS servers, with metrics moved from the http to a new gate subsystem.

I read through jsonrpc/gate.go, jsonrpc/websocket.go, jsonrpc/server.go, node/http.go, node/metrics.go, node/node.go, and cmd/juno/juno.go, and traced the concurrency/atomics in Gate (increment/decrement pairing on both success and failure paths, Running/Queued/Rejected accounting) plus the WS request loop (handleMessage correctly defers Release(), doesn't break the read loop on a "busy" write so the connection stays alive for subsequent messages, and drains the reader per the websocket library's read-to-EOF requirement). Tests (TestGateTryAcquire, TestWebsocketGateRejectsWhenBusy) exercise both the accept/reject/release cycle and the end-to-end WS busy path, and look correct. I wasn't able to execute go test/go build in this sandbox (network/tooling restrictions on the Rust VM deps and on invoking go test directly), so this is a static review only.

Findings: No correctness bugs, race conditions, or resource leaks found — the change is small, well-scoped, and well-tested.

• nit (not filed inline, already covered by gofmt/golangci-lint): jsonrpc/gate_test.go, jsonrpc/websocket_test.go, and node/metrics.go each have a stray double blank line before a new function, which gofmt -l flags. Worth a quick gofmt -w pass before merge since CI's lint step will likely catch it.

Nothing else stood out; PR looks clean overall.

[codecov]

Codecov Report

❌ Patch coverage is 80.39216% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.22%. Comparing base (f103674) to head (f137027).

Files with missing lines	Patch %	Lines
node/metrics.go	0.00%	6 Missing ⚠️
node/node.go	57.14%	3 Missing ⚠️
jsonrpc/websocket.go	96.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4014      +/-   ##
==========================================
- Coverage   79.29%   79.22%   -0.08%     
==========================================
  Files         465      465              
  Lines       35785    35815      +30     
==========================================
- Hits        28377    28374       -3     
- Misses       7399     7432      +33     
  Partials        9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0b04b73)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🏅 Score: 88

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Shared rejection counter logServerBusy logs ws.gate.Rejected(), which is the counter shared across HTTP and WebSocket (gate is now common to both). When both transports are active, a WebSocket busy-log line can report a rejected count that includes HTTP rejections too, making the log misleading about how many WebSocket requests specifically were refused. func (ws *Websocket) logServerBusy() { ws.sampledLogger.Warn("Rejected websocket RPC request: server is busy", zap.Int("running", ws.gate.Running()), zap.Int("queued", ws.gate.Queued()), zap.Uint64("rejected", ws.gate.Rejected()), ) }

[github-actions]

Claude finished @NazariiDenha's task in 6m 45s —— View job

---

Review: PR #4014 (Feat/ws rpc gate)

• Gather context (PR diff, prior review comments, CLAUDE.md)
• Review jsonrpc/gate.go + gate_test.go (TryAcquire correctness/races)
• Review jsonrpc/websocket.go + websocket_test.go (gate wiring, batch response format)
• Review jsonrpc/server.go (ServerBusy error code)
• Review node/http.go, node/node.go, node/metrics.go (shared gate wiring, metrics)
• Post inline comments for confirmed issues
• Post final summary

Summary: Extends the existing HTTP admission-control jsonrpc.Gate to WebSocket RPC via a new non-blocking TryAcquire(), shares one gate instance between HTTP and WS (created once in node/node.go), and adds a ServerBusy (-32004) JSON-RPC error for the WS path. TryAcquire/Release pairing, the activeRequests/sem bookkeeping, and the gate's nil-check pattern (mirroring the existing HTTP path) all look correct — no races or leaks found. Gate metrics are created exactly once regardless of which transports are enabled, avoiding duplicate Prometheus registration. Tests (TestGateTryAcquire, TestWebsocketGateRejectsWhenBusy) exercise the accept/reject/release cycle and the end-to-end WS busy path with real assertions (no flaky sleeps). gofmt is clean on all changed files.

Important

• jsonrpc/websocket.go:181-192 — when the gate is busy, handleMessage always writes a single-object serverBusyResponse, even if the incoming request was a JSON-RPC batch (array). Per spec, batch requests should get an array response; the non-busy path (HandleReadWriter) handles this correctly via isBatch. A batch-aware client that gets rejected while the gate is saturated over websocket receives a malformed (non-array) response. See inline comment for details/fix suggestion.

Nit

• node/metrics.go renames the RPC gate metrics from rpc_http_{active,queued,rejected}_requests to rpc_gate_{active,queued,rejected}_requests. This is a breaking change for any existing dashboards/alerts scraping the old rpc_http_* names — worth a callout in the PR description/changelog since it's a silent metric rename rather than an addition.

No blockers, concurrency issues, or resource leaks found otherwise. Given the PR title still says "NOT READY YET," these two points plus author's own remaining work are likely what's left before merge.

Note: I was unable to run go test/go build ./cmd/... in this sandbox — the full build requires prebuilt Rust libs (libjuno_starknet_compiler_rs, libjuno_starknet_rs) that aren't available here, and go test/golangci-lint invocations required approval that wasn't available in this run. go vet ./jsonrpc/... and gofmt -l on all changed files both passed cleanly. Static review only.

[github-actions]

Persistent review updated to latest commit 17c7218

[github-actions]

Claude finished @NazariiDenha's task in 3m 2s —— View job

---

Review: PR #4014 (Feat/ws rpc gate)

• Gather context (diff vs origin/main, prior review comments)
• Review jsonrpc/gate.go + gate_test.go
• Review jsonrpc/websocket.go + websocket_test.go
• Review jsonrpc/server.go
• Review node/http.go, node/node.go, node/metrics.go
• Post summary

What changed since the last review pass: only the fix lint commit (0b04b73) — removes stray blank lines in gate_test.go/websocket_test.go and tweaks two metric Help strings in node/metrics.go. No logic changed. gofmt -l is clean on all touched files and go vet ./jsonrpc/... passes.

Important (still open, not addressed by the latest commit)

• jsonrpc/websocket.go:181-192 — when gate.TryAcquire() fails, handleMessage always writes the single-object serverBusyResponse, even if the incoming payload is a JSON-RPC batch (top-level array). Per spec, batch requests must get an array response; the non-busy path (HandleReadWriter → isBatch) handles this correctly, the busy path does not. A batch-aware client hitting a saturated websocket gate gets a malformed (non-array) response. This was already flagged as an inline comment on this PR — noting it's still unresolved since the diff here is lint-only. See the existing inline comment for a fix sketch (peek the first non-whitespace byte of wsc.r, mirroring isBatch in jsonrpc/server.go, and wrap serverBusyResponse in [] when it's a batch).

No new issues found. Gate TryAcquire/Release accounting, the shared-gate wiring in node/node.go (created once, guarded by (cfg.HTTP || cfg.Websocket) && cfg.RPCMaxConcurrentRequests > 0 to avoid duplicate Prometheus registration), and the metrics rename to the gate subsystem all look correct — consistent with the prior review pass.

Note: go test/golangci-lint invocations require approval not available in this sandbox run, so this remains a static review (go vet and gofmt were run successfully).

[github-actions]

Persistent review updated to latest commit 0b04b73

[github-actions]

Claude finished @NazariiDenha's task in 3m 52s —— View job

---

Review: PR #4014 (Feat/ws rpc gate)

• Gather context (diff vs origin/main, prior review comments)
• Review jsonrpc/gate.go + gate_test.go
• Review jsonrpc/websocket.go + websocket_test.go
• Review jsonrpc/server.go
• Review node/http.go, node/node.go, node/metrics.go, cmd/juno/juno.go
• Post summary (no new inline comments needed)

What changed since the last review pass: nothing — HEAD is still f137027 (fix lint), same commit already reviewed. Re-verified the same diff against origin/main.

Verified findings from prior passes:

• jsonrpc/gate.go TryAcquire: the increaseActiveReq() / select / decreaseActiveReq() sequence is race-free (channel ops are goroutine-safe) and leaves activeRequests unchanged on failure — confirmed no leak, TestGateTryAcquire exercises accept → reject → release → reacquire correctly.
• jsonrpc/websocket.go handleMessage: gate acquired before HandleReadWriter, released via defer; on busy it writes serverBusyResponse and returns — the caller's io.Copy(io.Discard, wsc.r) in ServeHTTP still drains the unread message afterward, so the "read to EOF" websocket requirement is preserved even on the busy path. No goroutine/connection leak.
• The previously-flagged batch-shape issue (serverBusyResponse is always a single object, never an array) — I checked jsonrpc/server.go's existing HandleReader: "batch too large" and "batch requests are disabled" already return a single non-array object today (lines ~419–426), so the busy-gate response is consistent with existing, established precedent in this codebase rather than a new inconsistency. Reasonable to leave as-is per the author's response.
• node/node.go: the shared rpcGate is constructed exactly once, guarded by (cfg.HTTP || cfg.Websocket) && cfg.RPCMaxConcurrentRequests > 0, and metrics are registered once regardless of how many transports/servers consume it — no duplicate Prometheus registration.
• go vet ./jsonrpc/... and go build ./node/... both pass; gofmt -l is clean on all changed files. (go test still requires approval not available in this sandbox, so this remains primarily a static review plus vet/build.)

Nit (non-blocking, not a golangci-lint style issue): logServerBusy in both jsonrpc/http.go and jsonrpc/websocket.go logs gate.Rejected(), which is now a counter shared across HTTP and WS since the gate is common to both. A WS busy-log line's rejected count can include HTTP rejections and vice versa, which is slightly misleading for anyone reading logs per-transport. This was already raised by an earlier automated review pass and doesn't affect correctness — just a readability nit for whoever debugs from logs later.

No blockers, correctness bugs, races, or resource leaks found. The PR looks ready from a code-correctness standpoint.

[github-actions]

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 90

🧪 PR contains tests

🔒 No security concerns identified

⚡ No major issues detected