ChimeHQ / LanguageClient

Language Server Protocol (LSP) client for Swift
BSD 3-Clause "New" or "Revised" License
95 stars 10 forks source link

Server Never Receives Shutdown Response #22

Open thecoolwinter opened 3 weeks ago

thecoolwinter commented 3 weeks ago

I'm running up against an issue where the shutdownAndExit method is called on a server instance, but seems to wait forever after sending the .shutdown request.

After doing some digging, I think it might have to do with the language server sending null data in response to the shutdown message which doesn't trigger a data response in the JSONRPCServerConnection. I've come up with a minimal reproducing example using the sourcekit-lsp language server and attached it here. It's just a little swift executable that starts and then shuts down the server. When run, it starts and initializes the server but waits forever on the shutdownAndExit call.

I've also confirmed that both the initialize and shutdown messages are being successfully sent and encoded using some breakpoints. See attached screenshot.

Sent message contents

![Screenshot 2024-09-27 at 10 19 43 AM](https://github.com/user-attachments/assets/eb41f06e-8ebd-4fd9-8c5f-e7eda1284cb4)

Any help is appreciated!

LanguageClientShutdown.zip

thecoolwinter commented 3 weeks ago

It actually might've been the server I was using. Swapping out the server used in that example with the one bundled in Xcode 14 (xcrun --toolchain xcode -f sourcekit-lsp) I now receive the empty data response but am encountering a crash when decoding said data. The data received:

{"jsonrpc":"2.0","id":2,"result":{}}

The error is: Swift.DecodingError.typeMismatch(Swift.String, Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "result", intValue: nil)], debugDescription: "Expected to decode String but found a dictionary instead.", underlyingError: nil))

Screenshot 2024-09-27 at 10 48 21 AM

I'm going to see if swapping the String? decoded data with a dictionary fixes this in JSONRPCSession.swift line 239.

mattmassicotte commented 2 weeks ago

@thecoolwinter thanks for reporting this and for looking into it more closely. Some aspects of JSON-RPC were really hard to handle, and this could pontentialy be one of those cases.

This could totally end up being a bug/limitation of JSONRPC as well.

mattmassicotte commented 2 weeks ago

Also, one other thing. Are you encountering a crash or just a thrown error? I think I can reproduce the error with this input, but not a crash.

mattmassicotte commented 2 weeks ago

Ha! So returning "{}" is technically out of spec:

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#shutdown

thecoolwinter commented 2 weeks ago

Also, one other thing. Are you encountering a crash or just a thrown error? I think I can reproduce the error with this input, but not a crash.

In the example app I sent it's a crash cause of not handling errors on the top level. Otherwise it throws a decoding error into our error handlers.

I think the in-development version of sourcekit-lsp I was using just wasn't sending back the response so to fix that we'll just add a timeout to our shutdown call.

thecoolwinter commented 2 weeks ago

That could be something this library could handle but it seems like it could be out of scope. I'll leave that up to your discretion

mattmassicotte commented 2 weeks ago

Ok so there's at least two bugs here. The hang you were seeing is not expected. Something is going wrong with the state management here too. I think it is because of the timing between starting and attempting to stop the server but I'm not quite sure yet.

mattmassicotte commented 2 weeks ago

Ok, first things first: https://github.com/swiftlang/sourcekit-lsp/issues/1733

Now, I was able to work around the hang with by replacing the text document open notification with this: _ = try await server.initializeIfNeeded()

The hang is covered by this and I think I understand it: https://github.com/ChimeHQ/LanguageClient/issues/23 I also found this, which is not terrible, but stupid: https://github.com/ChimeHQ/LanguageServerProtocol/issues/23

And, now, finally we can get to the underlying issue. The problem is some types in LanguageServerProtocol, which are way too specific and kinda dumb. This will fix that:

https://github.com/ChimeHQ/LanguageServerProtocol/commit/f141d79365cd9cbeca83def76c4bc7566e1cb654

However, to get this fixed all the way, I have to make a release too and that's going to take just a little longer.