Skip to content
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

Fixes #5997 by intentionally sending Success responses from shutdown method #6007

Merged
merged 1 commit into from
Jun 28, 2021

Conversation

tm1000
Copy link
Contributor

@tm1000 tm1000 commented Jun 28, 2021

This Fixes #5997 by intentionally returning new Success(null) so that the yield does not throw because it's yielding on a null (which is an InvalidYieildError from Amp) Amp\\InvalidYieldError: Unexpected yield; Expected an instance of Amp\\Promise or React\\Promise\\PromiseInterface or an array of such instances; NULL yielded

Before Fix:

Message:

Content-Length: 60

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

Response

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"}

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

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

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

After Fix:

Message:

Content-Length: 60

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

Response

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"}

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

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

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

{"result":null,"id":4,"jsonrpc":"2.0"}

Because no error was thrown according to LSP my client now sends exit correctly.

You'll also notice that null was sent after shutdown succeeded which is correct according to the specification

@weirdan
Copy link
Collaborator

weirdan commented Jun 28, 2021

Thanks!

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.

Language Server Shutdown message throws exception
2 participants