Language Server Shutdown message throws exception

[tm1000]

Additionally the language server does not shutdown but continues to run in a broken state
Message:

Content-Length: 60

{"jsonrpc":"2.0","id":1,"method":"shutdown","params":[null]}

Response

out: Content-Type: application/vscode-jsonrpc; charset=utf8
Content-Length: 85

{"method":"telemetry\/event","params":{"type":3,"message":"closing"},"jsonrpc":"2.0"}Content-Type: application/vscode-jsonrpc; charset=utf8
Content-Length: 173

{"method":"window\/logMessage","params":{"type":4,"message":"[Psalm 4.8.1@f73f2299dbc59a3e6c4d66cff4605176e728ee69 - PHP Language Server] Shutting down..."},"jsonrpc":"2.0"}

out: Content-Type: application/vscode-jsonrpc; charset=utf8
Content-Length: 84

{"method":"telemetry\/event","params":{"type":3,"message":"closed"},"jsonrpc":"2.0"}

out: Content-Type: application/vscode-jsonrpc; charset=utf8
Content-Length: 3404

{"error":{"code":-32603,"message":"Amp\\InvalidYieldError: Unexpected yield; Expected an instance of Amp\\Promise or React\\Promise\\PromiseInterface or an array of such instances; NULL yielded at key 0 on line 133 in \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/src\/Psalm\/Internal\/LanguageServer\/LanguageServer.php in \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Coroutine.php:48\nStack trace:\n#0 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Coroutine.php(75): Amp\\Coroutine::transform(NULL, Object(Generator))\n#1 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/functions.php(96): Amp\\Coroutine->__construct(Object(Generator))\n#2 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/functions.php(61): Amp\\call(Object(Closure), Object(Psalm\\Internal\\LanguageServer\\Message))\n#3 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/src\/Psalm\/Internal\/LanguageServer\/EmitterTrait.php(78): Amp\\{closure}(Object(Psalm\\Internal\\LanguageServer\\Message))\n#4 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/src\/Psalm\/Internal\/LanguageServer\/ProtocolStreamReader.php(118): Psalm\\Internal\\LanguageServer\\ProtocolStreamReader->emit('message', Array)\n#5 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/src\/Psalm\/Internal\/LanguageServer\/ProtocolStreamReader.php(67): Psalm\\Internal\\LanguageServer\\ProtocolStreamReader->readMessages('{\"jsonrpc\":\"2.0...')\n#6 [internal function]: Psalm\\Internal\\LanguageServer\\ProtocolStreamReader->Psalm\\Internal\\LanguageServer\\{closure}()\n#7 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Coroutine.php(118): Generator->send('{\"jsonrpc\":\"2.0...')\n#8 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Internal\/Placeholder.php(149): Amp\\Coroutine->Amp\\{closure}(NULL, '{\"jsonrpc\":\"2.0...')\n#9 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Deferred.php(52): class@anonymous->resolve('{\"jsonrpc\":\"2.0...')\n#10 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/byte-stream\/lib\/ResourceInputStream.php(101): Amp\\Deferred->resolve('{\"jsonrpc\":\"2.0...')\n#11 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Loop\/NativeDriver.php(321): Amp\\ByteStream\\ResourceInputStream::Amp\\ByteStream\\{closure}('a', Resource id #1, NULL)\n#12 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Loop\/NativeDriver.php(127): Amp\\Loop\\NativeDriver->selectStreams(Array, Array, -0.001)\n#13 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Loop\/Driver.php(138): Amp\\Loop\\NativeDriver->dispatch(true)\n#14 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Loop\/Driver.php(72): Amp\\Loop\\Driver->tick()\n#15 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/amphp\/amp\/lib\/Loop.php(95): Amp\\Loop\\Driver->run()\n#16 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/src\/Psalm\/Internal\/Analyzer\/ProjectAnalyzer.php(533): Amp\\Loop::run()\n#17 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/src\/Psalm\/Internal\/Cli\/LanguageServer.php(312): Psalm\\Internal\\Analyzer\\ProjectAnalyzer->server(NULL, false)\n#18 \/Users\/andrewnagy\/smsapi-web\/app\/vendor\/vimeo\/psalm\/psalm-language-server(4): Psalm\\Internal\\Cli\\LanguageServer::run(Array)\n#19 {main}","data":null},"id":1,"jsonrpc":"2.0"}Content-Type: application/vscode-jsonrpc; charset=utf8

Since the error is caught the server does not shutdown

[psalm-github-bot]

Hey @tm1000, can you reproduce the issue on https://psalm.dev ?

[weirdan]

@tm1000 does your client also send exit notification? Spec says the server should not terminate until it receives exit request (https://microsoft.github.io/language-server-protocol/specifications/specification-current/#shutdown)

[tm1000]

@weirdan if I send exit then yes the server exits. The issue is that when sending shutdown there are only two responses that should be sent according to the language sever protocol (the same one you linked). That being either a null or an error. I get an error thrown then immediately analyzing messages. This means the shutdown command was not executed because of an error. In the case of my IDE (vscode) it will never send exit because shutdown returned an error followed by analyzing messages. Hence shutdown is broken.

I haven't tried yet but I believe shutdown needs to return the new Success(null) response at the end of the method instead of returning null to the yield.

Additionally I can cause this crash/error by sending any invalid message to the language server. I believe to fix this you just need to see if the response from dispatch is a null. If it's a null then return null instead of yielding null

[weirdan]

Can you provide steps to follow to reproduce the issue?

[tm1000]

@weirdan Sure. I'm also going to work up a PR for this as well

[weirdan]

Fab, thanks!

[tm1000]

@weirdan I tested #6007 and it successfully fixes the issues..

Additionally I work on https://github.com/psalm/psalm-vscode-plugin thanks to @muglug

[tm1000]

I had to do some Psalm related cleanup so I reissues the PR with cleaner commits.