Skip to content

fix(jsonrpc): preserve absolute columns in long parse errors - #4228

Open
zakazaka95 wants to merge 1 commit into
NethermindEth:mainfrom
zakazaka95:zakazaka95/fix-3914-absolute-columns
Open

zakazaka95 wants to merge 1 commit into
NethermindEth:mainfrom
zakazaka95:zakazaka95/fix-3914-absolute-columns

Conversation

@zakazaka95

Copy link
Copy Markdown

Continues #4020 following the invitation to resubmit, and addresses #3914 against current main (301cbb99).

Long requests can lose the beginning of a line from the diagnostic window, causing the reported column to restart inside the excerpt. Track discarded rune starts and trim at UTF-8 boundaries so the reported position remains absolute while the caret stays aligned with the displayed text. This retains current main’s JSON-v2 error handling and comma adjustment.

The three chunked cases now have literal expected JSON responses in a separate TestHandleParseErrorChunked, as requested in the earlier review. The long formatting expression is split as requested too.

Validation

Linux validation and benchmark run:

  • Full go test -short ./jsonrpc and go vet ./jsonrpc pass.
  • Both repository lint configurations pass for JSON-RPC, including strict changed-line checks; gofmt/gofumpt pass.
  • Restoring main’s formatter, removing absolute-prefix accounting, or using the absolute column for the displayed caret makes all three targeted cases fail. Restoring the fix passes again.
  • Dependencies and lockfiles are unchanged. No full-node or whole-repository test claim.

Requested benchmarks

Five runs per case, 2 seconds each, CPU=1 and GOMAXPROCS=1, on the same Linux runner. Values below are medians against 301cbb99.

Benchmark Main ns/op Fix ns/op Change B/op main → fix Allocs/op main → fix
BenchmarkHandle 3,586 3,596 +0.28% 1,036 → 1,039 18 → 18
LargeRequest/1MB 3,365,575 3,589,162 +6.64% 7,358,860 → 7,358,868 52 → 52
LargeRequest/10MB 26,696,681 27,707,726 +3.79% 65,035,847 → 65,035,849 58 → 58

These benchmarks use ASCII request bodies. The regression cases also cover mixed-width UTF-8 and a discarded newline. The extra prefix scan costs time on large requests; an ASCII-word fast path reduces that cost.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant