Show server crashed dialog on unexpected output in server's stdout #2296
Conversation
This file contains 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
rwols
approved these changes
Jul 6, 2023
This comment was marked as resolved.
This comment was marked as resolved.
|
I am wondering if we could print unexpected output to the LSP console,(so we will be aware that the server is returning something unexpected) That would fix #1612 |
|
Where do you draw the line? Such behavior is essentially not spec-compliant. I don't think we should be fixing or ignoring server issues like that. Also I would think that that specific eslint issue should be fixable on the server level. |
moetelo
pushed a commit
to moetelo/twiggy
that referenced
this pull request
Mar 20, 2024
LSP clients such as one for Sublime Text handle [anything non-JSON in language server output compliant as error](sublimelsp/LSP#2296) which is probably a good thing since we should expect proper output for language server client at all times, and any error should crash server. Twiggy language server uses output to print log information such as result of using PHP bin command which messes up JSON parser. This PR remaps console calls to connection calls, similar to how [Sass language server does it](https://github.com/wkillerud/some-sass/blob/eab00574065b210605013e3733f6ed971389c082/packages/language-server/src/node-server.ts#L7-L8). Addresses #7 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
On
int(headers.get("Content-Length"))crashing inJsonRpcProcessor.read_data, we've assumed that it's an expected case happening on server exit. As far as I've observed, the typical data we get on process exit is a newline, but it can also be any other random output that doesn't conform to the HTTP header format.In either of those cases we were just silently closing the transport, treating it as expected session shutdown.
Changed so that we only silently exit transport if the stdout was a single newline. In other cases propagate the output to the UI so that a crash dialog is shown.
I've also changed the
JSON decode errorcase to also propagate the error to the UI instead of silently ignoring payload. I think this makes more sense but we'd have to see how it works out in practice.Resolves #2295