Skip to content

Conversation

@HerringtonDarkholme
Copy link

The Language Server Protocol requires that all requests receive responses. Previously, ALE would silently ignore unknown JSON-RPC requests from language servers (like workspace/configuration), violating the LSP specification.

Solution:

  • Modified the central message handler in ale#lsp#HandleMessage to track whether any handler processed each message
  • Added handler return values: handlers now return v:true when they process server-initiated requests/notifications
  • Only 2 handlers needed modification since most handle client request responses:
    • ale#lsp_linter#HandleLSPResponse (textDocument/publishDiagnostics, etc.)
    • ale#codefix#HandleLSPResponse (workspace/applyEdit)
  • Unknown requests now receive standard JSON-RPC error responses with code -32601 (Method Not Found)

This ensures ALE is compliant with the LSP specification requirement that "Every processed request must send a response back to the sender."

🤖 Generated with Claude Code

Fixes #4610

I have not added vader test, due to insufficient AI training data. Yet I think this pull request is good enough for an inspiration.

HerringtonDarkholme and others added 2 commits September 4, 2025 18:45
The Language Server Protocol requires that all requests receive responses.
Previously, ALE would silently ignore unknown JSON-RPC requests from language
servers (like workspace/configuration), violating the LSP specification.

Solution:
- Modified the central message handler in ale#lsp#HandleMessage to track
  whether any handler processed each message
- Added handler return values: handlers now return v:true when they process
  server-initiated requests/notifications
- Only 2 handlers needed modification since most handle client request responses:
  * ale#lsp_linter#HandleLSPResponse (textDocument/publishDiagnostics, etc.)
  * ale#codefix#HandleLSPResponse (workspace/applyEdit)
- Unknown requests now receive standard JSON-RPC error responses with code
  -32601 (Method Not Found)

This ensures ALE is compliant with the LSP specification requirement that
"Every processed request must send a response back to the sender."

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
The Language Server Protocol requires that all requests receive responses.
Previously, ALE would silently ignore unknown JSON-RPC requests from language
servers (like workspace/configuration), violating the LSP specification.

Solution:
- Modified the central message handler in ale#lsp#HandleMessage to track
  whether any handler processed each message
- Added handler return values: handlers now return v:true when they process
  server-initiated requests/notifications
- Only 2 handlers needed modification since most handle client request responses:
  * ale#lsp_linter#HandleLSPResponse (textDocument/publishDiagnostics, etc.)
  * ale#codefix#HandleLSPResponse (workspace/applyEdit)
- Unknown requests now receive standard JSON-RPC error responses with code
  -32601 (Method Not Found)

This ensures ALE is compliant with the LSP specification requirement that
"Every processed request must send a response back to the sender."

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@hsanson hsanson requested a review from w0rp September 6, 2025 07:35
@rymdbar rymdbar mentioned this pull request Sep 8, 2025
@rymdbar
Copy link
Contributor

rymdbar commented Sep 19, 2025

Vibe coding it, like it's 2025! Well done.

I've pushed rymdbar:pr/5044 which adds two, hand-crafted LLM-free, details:

  • A trivial message informing the user an unknown call was made.
  • An attempt at a Vader test case detecting the above message.

One could imagine the user might be interesting in learning the names of unknown calls, so that might be a possible improvement.

The test case kind of works, but is a bit of a hack. I assume there are better ways of doing it, but this is what I came up with.

What worries me most is that merging this with my commit included might lead to users feeling flooded by messages caused by lsp:s ignoring announced capabilities and just yolo:ing calls.

Please feel free updating the PR to include this test case.

@benknoble
Copy link
Contributor

I've pushed rymdbar:pr/5044 which adds two, hand-crafted LLM-free, details:

  • A trivial message informing the user an unknown call was made.

[snip]

What worries me most is that merging this with my commit included might lead to users feeling flooded by messages caused by lsp:s ignoring announced capabilities and just yolo:ing calls.

What if we put this behind a setting (g:ale…), and also included such errors in the :ALEInfo output? I for one would probably enable it so I can report server or client bugs (or to debug things), but I agree with worrying about the flood.

  • An attempt at a Vader test case detecting the above message.

[snip]

The test case kind of works, but is a bit of a hack. I assume there are better ways of doing it, but this is what I came up with.

I think there are test cases that simulate server requests and check responses?

One could imagine the user might be interesting in learning the names of unknown calls, so that might be a possible improvement.

❤️

@HerringtonDarkholme
Copy link
Author

Hi ale team, I personally don't have time to add tests. But feel free to modify this PR or take the code to create a new PR. Cheers!

@Spixmaster
Copy link
Contributor

Spixmaster commented Oct 30, 2025

I just tested this pull request after the recommendation from @rymdbar.

This fixes #4998, #5058 and biomejs/biome#7908.

Can we get this merged?

Copy link
Contributor

@benknoble benknoble left a comment

Choose a reason for hiding this comment

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

Just a nit.

Comment on lines -400 to +406
call ale#util#GetFunction(l:Callback)(a:conn_id, l:response)
let l:result = ale#util#GetFunction(l:Callback)(a:conn_id, l:response)

if l:result is v:true
let l:handled = 1
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Simplify: if ale#util…

@rymdbar
Copy link
Contributor

rymdbar commented Oct 31, 2025

I just tested this pull request after the recommendation from @rymdbar.

My exact words were: You might wish to look at the conversation in that pull request and chip in your code to complete it?

Looking at the pull request was only half of the instruction.

Can we get this merged?

You mean in its current state? I think the mere fact that this pull request is open already answered that before the question was phrased.

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.

Unknown json-rpc requests should get an error response, not be ignored

4 participants