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

Clarification: re-order responses? #306

Closed
ljw1004 opened this issue Sep 23, 2017 · 3 comments
Closed

Clarification: re-order responses? #306

ljw1004 opened this issue Sep 23, 2017 · 3 comments
Labels
*question Issue represents a question, should be posted to StackOverflow (VS Code)

Comments

@ljw1004
Copy link
Contributor

ljw1004 commented Sep 23, 2017

The LSP spec says [link]

Responses for requests should be sent in the same order as the requests appear on the server or client side. So for example if a server receives a textDocument/completion request and then a textDocument/signatureHelp request it should first return the response for the textDocument/completion and then the response for textDocument/signatureHelp.

How the server internally processes the requests is up to the server implementation. If the server decides to execute them in parallel and this produces correct result the server is free to do so. The server is also allowed to reorder requests and notification if the reordering doesn't affect correctness.

This is confusing -- e.g. ocaml/merlin#607 (comment)

Q. If the client sends textDocument/definition request id 10 followed by textDocument/hover request id 11, is the LSP server permitted to send back response id 11 followed by response id 10 if this still "produces the correct result"?

The spec says the server is able to "reorder requests and notifications if the reordering doesn't affect correctness". I don't know if this means to allow arbitrary re-ordering of requests with respect to each other, or if it's merely to allow re-ordering of notifications with respect to requests.

cc. @freebroccolo

@rcjsuen
Copy link
Contributor

rcjsuen commented Sep 23, 2017

May also want to see #278.

@object88
Copy link
Contributor

object88 commented Sep 27, 2017

The way that I read it, should implies that it's a suggested, but not a hard requirement. It seems to be the intent of the protocol to guide behaviors rather than prescribe hard & fast rules where not necessary. While the spec makes this a recommendation, it may be in the client's interest to receive the responses in order. (I.e., perhaps one request supersedes another, and it's easiest to just drop a request that isn't current.)

The particular note about reorder[ing] seems to only refer to the server's internal processing, not sending the response. I interpret that as, if the server processes requests asynchronously or out-of-order, it should be able to reorder the responses themselves.

@dbaeumer
Copy link
Member

dbaeumer commented Oct 3, 2017

@object88 is correct. The protocol only defines that results must be correct in terms of its request.

Since the whole protocol is fully async it is very hard to define strict ordering anyways. For example in node if you create two promises there is IMO no guarantee that the first created promises resolves first and the second promise second even if they are constructed with a resolved value.

Since the client sends requests in a specific order it is nice if the server returns them in the same order since results are usually shown in the UI as a result of user actions and a user executed them in a certain order. However given the above the UI might not react in the same order.

I am open for a better description if you think that the current sentences don't make this clear enough. Please add them here and I will merge them in.

@dbaeumer dbaeumer closed this as completed Oct 3, 2017
@dbaeumer dbaeumer added the *question Issue represents a question, should be posted to StackOverflow (VS Code) label Oct 3, 2017
dbaeumer added a commit that referenced this issue Nov 15, 2017
Clarify response ordering (#306)
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*question Issue represents a question, should be posted to StackOverflow (VS Code)
Projects
None yet
Development

No branches or pull requests

4 participants