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

How to abort during initialize? #246

Open
ljw1004 opened this issue May 19, 2017 · 3 comments
Open

How to abort during initialize? #246

ljw1004 opened this issue May 19, 2017 · 3 comments
Labels
feature-request Request for new features or functionality initialization
Milestone

Comments

@ljw1004
Copy link
Contributor

ljw1004 commented May 19, 2017

How can the client abort/cancel/shutdown during initialization?

Our language service sometimes takes ~5 minutes to start up, during which time it's unable to successfully handle any requests or notifications. Because if this I decided to make the "initialize" method only respond once it has finished starting up [see at end for explanation why].

But how to handle the case when the client wants to abandon the initialization part way through?

  • I imagine the client could send a $/cancelRequest notification to cancel the initialize request. This is allowed by JsonRpc, but LSP says the only notification allowed before the initialize request is exit, and other notifications should be dropped, and the client must not send additional notifications until the server has responded to initialize.

  • I imagine the client could send a shutdown request. The LSP spec says that the server must respond with code -32002 if this (or any) request comes before the initialize request, and says the client must not send any requests until after the initialize response.

  • I imagine the client could send a exit notification. This would work in theory according to the LSP spec. But the way LanguageClient is implemented, when you call stop(), it first sends a shutdown request, and only if that succeeds will it send the exit notification. So this plan won't work well. Indeed, if LanguageClient is asked to stop() in the middle of initialization, it will transition into the Stopping state and not dispose of anything or clean anything up.

https://github.com/Microsoft/vscode-languageserver-node/blob/master/client/src/client.ts#L1530

Why can't the server respond immediately to the initialize request? ... If it responded to initialize immediately, then the client would merrily send loads of "didOpen/didChange" notifications, and the server would have no way of indicating that it wasn't ready for them, so it would have to queue every single such notification up until such time as it could handle them. That'd be crummy. Dynamic registration might notionally work, but I don't think it's good to rely on clients supporting it.

@ljw1004
Copy link
Contributor Author

ljw1004 commented May 19, 2017

Digging further into the code, I've come to a different conclusion for LSP servers that don't depend upon dynamic registration:

  1. An LSP server which takes a long time to initialize should actually respond to Initialize method promptly.
  2. It should respond to shutdown request and exit notification in the normal way.
  3. It should respond to all other requests with RequestCancelled error code. (This is the only error code that won't show up in the VSCode output window).
  4. It should queue up all didOpen/didChange/didClose notifications to handle at a time when it's ready, since these affect the 'truth' of documents and won't be re-sent in future. All other notifications should be ignored.
  5. It should accept responses as normal.

Why should it respond to the Initialize request promptly?

  • In the current LanguageClient codebase, if you try to stop() a language server while its initialize request is still being processed, then the stop method won't do anything until after initialize has finally finished. It's bad if you can't use stop() properly.
  • In the current LanguageClient codebase, if the LSP server should encounter a problem and stop while it's processing the initialize method, then LanguageClient will have both the "initialize retry logic" and the "handleConnectionClosed automatic restart logic" both running together, and neither was designed with this scenario in mind.

@dbaeumer
Copy link
Member

@ljw1004 I agree with your conclusion of sending the initialize response as soon as possible.

What we could think about having a Language Server status item and add this to the LSP so that the server can let the client know about its status. This could even show progress until the initialization is done. However the user experience would still not be very good here since for example code compete would present and empty list (or nothing) without letting the user know that the server is not ready yet.

@dbaeumer dbaeumer added the feature-request Request for new features or functionality label Jun 15, 2017
@dbaeumer dbaeumer added this to the Backlog milestone Jun 15, 2017
@ljw1004
Copy link
Contributor Author

ljw1004 commented Jun 15, 2017

Another option: instead of a Language Server status item, it could instead be handled with a ResponseError - maybe a new code ServerNotYetReady. That way it'd be up to the client to decide how to display this to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality initialization
Projects
None yet
Development

No branches or pull requests

2 participants