Skip to content

Fix invalid params error handling#2764

Merged
gabritto merged 12 commits intomainfrom
gabritto/fix-error-id
Feb 12, 2026
Merged

Fix invalid params error handling#2764
gabritto merged 12 commits intomainfrom
gabritto/fix-error-id

Conversation

@gabritto
Copy link
Member

Found this while testing the fuzzer: if we failed to unmarshal params for a request, we weren't really sending the error message with the proper id (this was causing the fuzzer to hang). We were also using error code invalid request even when we should the more specific invalid params error.

Copilot AI review requested due to automatic review settings February 11, 2026 22:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes error handling for invalid JSON-RPC request parameters in the LSP server. Previously, when parameter unmarshaling failed, the server was using the generic InvalidRequest error code and wasn't properly preserving the request ID in the error response, which could cause issues like the fuzzer hanging.

Changes:

  • Changed error code from InvalidRequest to the more specific InvalidParams when parameter unmarshaling fails
  • Modified error handling to preserve and return the request ID in error responses for invalid params
  • Added test coverage to verify the fix works correctly

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
internal/lsp/lsproto/jsonrpc.go Moved error check after Message construction to allow ID extraction, changed error code to InvalidParams
internal/lsp/server.go Added logic to detect InvalidParams errors and extract request ID from the partially unmarshaled message for error responses
internal/lsp/server_invalid_message_test.go Added test to verify that invalid params trigger the correct error code and response includes the request ID

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
type ResponseMessage struct {
JSONRPC jsonrpc.JSONRPCVersion `json:"jsonrpc"`
ID *jsonrpc.ID `json:"id,omitzero"`
ID *jsonrpc.ID `json:"id"`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the spec, if we fail to decode a request or notification (i.e. the whole thing, not just params), we are allowed to return an error response with null id.

@gabritto gabritto requested a review from jakebailey February 12, 2026 22:46
@gabritto gabritto enabled auto-merge February 12, 2026 23:02
@gabritto gabritto added this pull request to the merge queue Feb 12, 2026
Merged via the queue into main with commit fbcae84 Feb 12, 2026
20 checks passed
@gabritto gabritto deleted the gabritto/fix-error-id branch February 12, 2026 23:25
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.

3 participants