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

Reset CompletionState when textDocument/completion gets an ResponseError #131

Closed
randy3k opened this issue Sep 29, 2017 · 10 comments
Closed

Comments

@randy3k
Copy link
Contributor

randy3k commented Sep 29, 2017

We need to reset CompletionState to IDLE, otherwise, autocompletion will be frozen.

@deathaxe
Copy link
Contributor

deathaxe commented Oct 3, 2017

The problem is caused by a very general one. If ANY request is responded with a server error -32000 the response handler is never called. Therefore the CompletionHandler's response_handler in this case is never called. The error response seems not to include the request id?

Therefore the CompletionHandler gets caught in CANCALLING state and can't recover.

@tomv564
Copy link
Contributor

tomv564 commented Oct 3, 2017

There is some requirement that responses arrive in the same order that the requests are made: microsoft/language-server-protocol#12

How common should it be for completion requests to return an error? Errors should be shown in the status bar already, the user can always restart the server re-open the document to recover.

@randy3k, what language server is returning the error, and what is the error returned?

@randy3k
Copy link
Contributor Author

randy3k commented Oct 3, 2017

I encountered the error when developing the R language server. They are runtime errors due to bugs in completion handler where InternalError is returned.

@tomv564
Copy link
Contributor

tomv564 commented Oct 3, 2017

Closing and reopening the view should set up a new CompletionHandler with fresh state.

Let me know if that is not a functional or acceptable workaround for what should be rare or development-time language server errors.

@deathaxe
Copy link
Contributor

deathaxe commented Oct 4, 2017

No software is perfect. We always should assume something to possibly go wrong. Therefore I think LSP should handle error responses somehow and in this case really cancel the completion. How should a normal user find out why completions suddenly stopped working? I was indeed wondering for a while, too, until I found the error responses to be the reason. See the 'column parameter is not in a valid range.' issue from #140 as an example. A server always is allowed to respond with an error - doesn't need to be bug triggered. It should not cause LSP to stop working. I am absolutely with you, "id" would need to be sent back as a reference to the request the response belongs to - even in error situation.

But with the assumption of ordered request/response chains, this issue might be solved by adding some kind of fifo and remember all sent requests until the corresponding response was received to catch errors as well and forward them to the response handlers.

@randy3k
Copy link
Contributor Author

randy3k commented Oct 4, 2017

I am with @deathaxe. I think in general we need to pass the responses to the corresponding handler without catching them too early. It is not a solution at all to re-open the document if it can be avoided.

@tomv564
Copy link
Contributor

tomv564 commented Oct 4, 2017

Yep, we can keep this issue for later.
Right now I think there are more important issues, like #140.

@randy3k
Copy link
Contributor Author

randy3k commented Oct 5, 2017

I thought #140 and this are caused by the same bug.

@tomv564
Copy link
Contributor

tomv564 commented Oct 5, 2017

You are correct, but this bug can have many cases in the future and needs to be fixed separately in the future.

@deathaxe
Copy link
Contributor

deathaxe commented Oct 5, 2017

Yes this is a more general issue. This topic is just an example. I guess it needs some time to work out a strategy and may cause bigger changes to solve this issue cleanly.

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

No branches or pull requests

3 participants