-
Notifications
You must be signed in to change notification settings - Fork 235
feat(jsonrpc): clearer errors for malformed JSON and invalid params #3702
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
00a335a
feat(jsonrpc): clearer errors for malformed JSON and invalid params
RafaelGranza c03938d
refactor(jsonrpc): accurate parse positions and benchmark
RafaelGranza 779610b
docs(jsonrpc): drop redundant isBatch comment
RafaelGranza 71849ea
chore(jsonrpc): only skip lll on server_test.go
RafaelGranza eb5b4fc
feat(jsonrpc): mark errors
RafaelGranza fd1a11a
feat(jsonrpc): Pyhton like error
RafaelGranza 0f02636
style(jsonrpc): address lint suggestion
RafaelGranza 5f8931f
test(jsonrpc): Extra tests
RafaelGranza 4f4b817
refactor(jsonrpc): polish parse error messages
RafaelGranza 03affb8
feat(jsonrpc): horizontal trim and sliding-window error buffer
RafaelGranza 8d424c3
test(jsonrpc): Add extra tests
RafaelGranza 9794edf
test(jsonrpc): realistic JSON in parse errors
RafaelGranza ad165d0
test: Address requests
RafaelGranza c48abb4
refactor(jsonrpc): tidy windowBuffer write
RafaelGranza a903f34
test(jsonrpc): split out parse error tests
RafaelGranza 59c0b0b
ci: extend lll exemption to pretty_error_test
RafaelGranza File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,207 @@ | ||
| package jsonrpc | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "slices" | ||
| "strings" | ||
| "unicode/utf8" | ||
| ) | ||
|
|
||
| const ( | ||
| maxWindowSize = 512 | ||
| maxLineWidth = 80 | ||
| maxContextRows = 3 | ||
| ) | ||
|
|
||
| // windowBuffer keeps the last maxWindowSize bytes read | ||
| type windowBuffer struct { | ||
| window []byte | ||
| consumedBytes int | ||
| newlinesSeen int | ||
| } | ||
|
|
||
| func (c *windowBuffer) Write(p []byte) (int, error) { | ||
| c.consumedBytes += len(p) | ||
| c.newlinesSeen += bytes.Count(p, []byte{'\n'}) | ||
|
|
||
| if len(p) >= maxWindowSize { | ||
| c.window = append(c.window[:0], p[len(p)-maxWindowSize:]...) | ||
| return len(p), nil | ||
|
rodrodros marked this conversation as resolved.
rodrodros marked this conversation as resolved.
|
||
| } | ||
|
|
||
| if overflow := len(c.window) + len(p) - maxWindowSize; overflow > 0 { | ||
| c.window = c.window[:copy(c.window, c.window[overflow:])] | ||
| } | ||
| c.window = append(c.window, p...) | ||
|
|
||
| return len(p), nil | ||
| } | ||
|
|
||
| func errorOffset(inputLength int, err error) (offset int, ok bool) { | ||
| var ( | ||
| syntaxErr *json.SyntaxError | ||
| typeErr *json.UnmarshalTypeError | ||
| ) | ||
| switch { | ||
| case errors.As(err, &syntaxErr): | ||
| return min(int(syntaxErr.Offset)-1, inputLength), true | ||
| case errors.As(err, &typeErr): | ||
| return min(int(typeErr.Offset)-1, inputLength), true | ||
| case errors.Is(err, io.ErrUnexpectedEOF), errors.Is(err, io.EOF): | ||
| return inputLength, true | ||
| default: | ||
| // TODO(granza): when we add SONIC, the errors will be already pretty. | ||
| return 0, false | ||
| } | ||
| } | ||
|
|
||
| func lineAndColumn(c *windowBuffer, markerPos int) (line, col int) { | ||
| line = c.newlinesSeen - bytes.Count(c.window[markerPos:], []byte{'\n'}) + 1 | ||
| lineStart := bytes.LastIndexByte(c.window[:markerPos], '\n') + 1 | ||
| col = utf8.RuneCount(c.window[lineStart:markerPos]) + 1 | ||
| return line, col | ||
| } | ||
|
RafaelGranza marked this conversation as resolved.
|
||
|
|
||
| func expectedToken(reason string) (string, bool) { | ||
| // The stdlib reason uses parser jargon | ||
| // This maps it to user-friendly messages | ||
| switch { | ||
| case strings.Contains(reason, "beginning of object key string"): | ||
| return "a string key or '}'", true | ||
| case strings.Contains(reason, "after object key:value pair"): | ||
| return "',' or '}'", true | ||
| case strings.Contains(reason, "after object key"): | ||
| return "':'", true | ||
| case strings.Contains(reason, "after array element"): | ||
| return "',' or ']'", true | ||
| case strings.Contains(reason, "beginning of value"): | ||
| return "a value", true | ||
| default: | ||
| return "", false | ||
| } | ||
| } | ||
|
|
||
| func precededByComma(input []byte, offset int) bool { | ||
| trimmed := bytes.TrimRight(input[:offset], " \t\r\n") | ||
| return len(trimmed) > 0 && trimmed[len(trimmed)-1] == ',' | ||
| } | ||
|
|
||
| func describeTypeError(e *json.UnmarshalTypeError) string { | ||
| if e.Field != "" { | ||
| return fmt.Sprintf("field %q should be %s, got %s", e.Field, e.Type, e.Value) | ||
| } | ||
| return fmt.Sprintf("expected a JSON object, got %s", e.Value) | ||
| } | ||
|
|
||
| func describeSyntaxError(input []byte, offset int, err error) string { | ||
| expected, ok := expectedToken(err.Error()) | ||
| if !ok || offset >= len(input) { | ||
| return err.Error() | ||
| } | ||
| symbol, _ := utf8.DecodeRune(input[offset:]) | ||
| if (symbol == '}' || symbol == ']') && precededByComma(input, offset) { | ||
| return fmt.Sprintf("unexpected trailing comma before %q", symbol) | ||
| } | ||
| return fmt.Sprintf("unexpected %q, expected %s", symbol, expected) | ||
| } | ||
|
|
||
| func describeError(input []byte, offset int, err error) string { | ||
| var ( | ||
| syntaxErr *json.SyntaxError | ||
| typeErr *json.UnmarshalTypeError | ||
| ) | ||
| switch { | ||
| case errors.As(err, &syntaxErr): | ||
| return describeSyntaxError(input, offset, err) | ||
| case errors.As(err, &typeErr): | ||
| return describeTypeError(typeErr) | ||
| case errors.Is(err, io.ErrUnexpectedEOF), errors.Is(err, io.EOF): | ||
| return "unexpected end of input" | ||
| default: | ||
| return err.Error() | ||
|
RafaelGranza marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| func offendingLine(input []byte, offset int) string { | ||
| start := bytes.LastIndexByte(input[:offset], '\n') + 1 | ||
| if end := bytes.IndexByte(input[offset:], '\n'); end >= 0 { | ||
| return string(input[start : offset+end]) | ||
| } | ||
| return string(input[start:]) | ||
| } | ||
|
|
||
| // Returns the truncated string around a pivot and the new index of it | ||
| func truncateAround(line string, pivot, maxLineWidth int) (string, int) { | ||
| runes := []rune(line) | ||
| if len(runes) <= maxLineWidth { | ||
| return line, pivot | ||
| } | ||
|
|
||
| const ellipsis = "..." | ||
| maxContextSize := maxLineWidth - 2*len(ellipsis) | ||
| pivotIdx := min(pivot-1, len(runes)) | ||
| start := max(0, pivotIdx-maxContextSize/2) | ||
| end := min(start+maxContextSize, len(runes)) | ||
| start = max(0, end-maxContextSize) | ||
|
|
||
| left, right := "", "" | ||
| if start > 0 { | ||
| left = ellipsis | ||
| } | ||
| if end < len(runes) { | ||
| right = ellipsis | ||
| } | ||
| return left + string(runes[start:end]) + right, pivotIdx - start + len(left) + 1 | ||
| } | ||
|
|
||
| func precedingLines(window []byte, windowStart, markerPos int) []string { | ||
| lineStart := bytes.LastIndexByte(window[:markerPos], '\n') + 1 | ||
|
|
||
| var rows []string | ||
| for len(rows) < maxContextRows && lineStart > 0 { | ||
| prevEnd := lineStart - 1 | ||
| prevStart := bytes.LastIndexByte(window[:prevEnd], '\n') + 1 | ||
| if prevStart == 0 && windowStart > 0 { | ||
| break // the topmost line was cut off by the window | ||
| } | ||
|
|
||
| row, _ := truncateAround(string(window[prevStart:prevEnd]), 1, maxLineWidth) | ||
| rows = append(rows, row) | ||
| lineStart = prevStart | ||
| } | ||
|
|
||
| slices.Reverse(rows) | ||
| return rows | ||
| } | ||
|
|
||
| func drawMarker(window []byte, windowStart, markerPos, col int, msg string) string { | ||
| var fullMsg strings.Builder | ||
| for _, row := range precedingLines(window, windowStart, markerPos) { | ||
| fullMsg.WriteString(row) | ||
| fullMsg.WriteByte('\n') | ||
| } | ||
|
|
||
| line, markerCol := truncateAround(offendingLine(window, markerPos), col, maxLineWidth) | ||
| gap := strings.Repeat(" ", markerCol-1) | ||
|
|
||
| fmt.Fprintf(&fullMsg, "%s\n%s^\n%s", line, gap, msg) | ||
| return fullMsg.String() | ||
| } | ||
|
|
||
| func prettyParseError(c *windowBuffer, err error) string { | ||
| absOffset, ok := errorOffset(c.consumedBytes, err) | ||
| if !ok { | ||
| return err.Error() | ||
| } | ||
|
|
||
| windowStart := c.consumedBytes - len(c.window) | ||
| markerPos := max(0, absOffset-windowStart) | ||
| line, col := lineAndColumn(c, markerPos) | ||
| msg := fmt.Sprintf("%s [line %d, position %d]", describeError(c.window, markerPos, err), line, col) | ||
|
|
||
| return drawMarker(c.window, windowStart, markerPos, col, msg) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.