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

Inspect ClientCapabilities.general.staleRequestSupport to decide whether to cancel requests on edit #1650

Open
ahoppen opened this issue Sep 7, 2024 · 1 comment

Comments

@ahoppen
Copy link
Member

ahoppen commented Sep 7, 2024

I just realized that there is a staleRequestSupport section in the client capabilities in LSP:

/**
 * Client capability that signals how the client
 * handles stale requests (e.g. a request
 * for which the client will not process the response
 * anymore since the information is outdated).
 *
 * @since 3.17.0
 */
staleRequestSupport?: {
  /**
   * The client will actively cancel the request.
   */
  cancel: boolean;

  /**
   * The list of requests for which the client
   * will retry the request if it receives a
   * response with error code `ContentModified``
   */
   retryOnContentModified: string[];
}

VS Code sends the following options

"staleRequestSupport": {
    "cancel": true,
    "retryOnContentModified": [
        "textDocument/semanticTokens/full",
        "textDocument/semanticTokens/range",
        "textDocument/semanticTokens/full/delta"
    ]
}

I am still unsure how to interpret the retryOnContentModified section because the ContentModified error code is described as follows

/**
 * The server detected that the content of a document got
 * modified outside normal conditions. A server should
 * NOT send this error code if it detects a content change
 * in it unprocessed messages. The result even computed
 * on an older state might still be useful for the client.
 *
 * If a client decides that a result is not of any use anymore
 * the client should cancel the request.
 */
export const ContentModified: integer = -32801;

which indicates that LSP servers should not cancel requests when they detect an edit to the document, contradicting the comment in staleRequestSupport IMO. The commit that introduced staleRequestSupport also isn’t very insightful: microsoft/language-server-protocol@8aec48a

Based on VS Code’s runtime behavior, I guess that it’s intended as follows:

  • staleRequestSupport.cancel: true indicates that the LSP server can generally rely on the client cancelling stale requests and doesn’t need to do any implicit cancellation
  • staleRequestSupport.retryOnContentModified means that there is a list of requests that the client does not cancel. For these, it relies on the server replying with ContentModified if it detects a content change, even though that contradicts the definition of the ContentChange error code.

So, maybe the correct way of implementing implicit cancellation is:

  • If the client does not set staleRequestSupport in the client capabilities or if staleRequsetSupport.cancel is false, we should cancel requests on document edits (the behavior we currently get with cancelTextDocumentRequestsOnEditAndClose: true). I am not sure if the error code that we return here matters.
  • If staleRequestSupport.cancel is true then only document requests in staleRequestSupport.retryOnContentModified should be implicitly cancelled on document edits, returning a ContentModified error code.
  • I think it’s still valuable to have a cancelTextDocumentRequestsOnEditAndClose configuration setting to override the behavior to always cancel requests on edits, forcing the same behavior as if staleRequestSupport was not set in the client capabilities. This configuration setting should default to false.
@ahoppen
Copy link
Member Author

ahoppen commented Sep 7, 2024

Synced to Apple’s issue tracker as rdar://135476608

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

1 participant