Fix invalid client/registerCapability request #2436

rchl · 2020-11-05T10:46:46Z

Per current version of the spec [1], the "documentSelector" is a list of
DocumentFilter and can not contain a string.

It seems like it was allowed to have a string in the past [2] but it's
no longer the case and it doesn't work even in VSCode.

[1] https://microsoft.github.io/language-server-protocol/specification#documentFilter
[2] microsoft/vscode-languageserver-node#685

Resolves #2388

rchl · 2020-11-05T11:33:13Z

I found this issue initially in Sublime Text but trying latest release (0.29.0) in VSCode, it is also failing there:

Details

[Trace - 12:02:59 PM] Received notification 'window/logMessage'.
Params: {
    "type": 3,
    "message": "Registering request handler for textDocument/formatting failed."
}


[Info  - 12:02:59 PM] Registering request handler for textDocument/formatting failed.
[Trace - 12:02:59 PM] Received notification 'window/logMessage'.
Params: {
    "type": 1,
    "message": "(node:60431) UnhandledPromiseRejectionWarning: Error: Unhandled method client/registerCapability\n    at handleResponse (/Users/me/.vscode/extensions/octref.vetur-0.29.0/server/node_modules/vscode-jsonrpc/lib/common/connection.js:476:48)\n    at processMessageQueue (/Users/me/.vscode/extensions/octref.vetur-0.29.0/server/node_modules/vscode-jsonrpc/lib/common/connection.js:291:17)\n    at Immediate.<anonymous> (/Users/me/.vscode/extensions/octref.vetur-0.29.0/server/node_modules/vscode-jsonrpc/lib/common/connection.js:275:13)\n    at processImmediate (internal/timers.js:439:21)"
}


[Error - 12:02:59 PM] (node:60431) UnhandledPromiseRejectionWarning: Error: Unhandled method client/registerCapability
    at handleResponse (/Users/me/.vscode/extensions/octref.vetur-0.29.0/server/node_modules/vscode-jsonrpc/lib/common/connection.js:476:48)
    at processMessageQueue (/Users/me/.vscode/extensions/octref.vetur-0.29.0/server/node_modules/vscode-jsonrpc/lib/common/connection.js:291:17)
    at Immediate.<anonymous> (/Users/me/.vscode/extensions/octref.vetur-0.29.0/server/node_modules/vscode-jsonrpc/lib/common/connection.js:275:13)
    at processImmediate (internal/timers.js:439:21)
[Trace - 12:02:59 PM] Received notification 'window/logMessage'.
Params: {
    "type": 1,
    "message": "(node:60431) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)"
}


[Error - 12:02:59 PM] (node:60431) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
[Trace - 12:02:59 PM] Received notification 'window/logMessage'.
Params: {
    "type": 1,
    "message": "(node:60431) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code."
}


[Error - 12:02:59 PM] (node:60431) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
[Trace - 12:02:59 PM] Received request 'client/registerCapability - (1)'.
Params: {
    "registrations": [
        {
            "id": "a43c54a4-a137-46ad-b926-2c26f3dbd6d9",
            "method": "textDocument/formatting",
            "registerOptions": {
                "documentSelector": [
                    "vue"
                ]
            }
        }
    ]
}


[Trace - 12:02:59 PM] Sending response 'client/registerCapability - (1)'. Processing request took 1ms
No result returned.

yoyo930021 · 2020-11-05T12:14:04Z

This also explains this problem.
tommasongr/nova-vue#11 (comment)

But I think it's reserved for old version compatibility.
And vetur have same code in

vetur/client/client.ts

Line 40 in 1851d1c

documentSelector,

rchl · 2020-11-05T13:33:27Z

I've updated that one too and also rewritten the commit message to better describe the problem.

Decided to not remove the selector but just update it to a spec-compatible format since that is safer.

J3m5 · 2020-11-06T10:42:13Z

I built it and pointed the extension to the build.
The params are well formatted but I still get the error:

[Error - 11:34:26 AM] (node:72001) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
[Trace - 11:34:26 AM] Received request 'client/registerCapability - (1)'.
Params: {
    "registrations": [
        {
            "id": "602a3fae-6760-40ca-a6aa-52851f674b37",
            "method": "textDocument/formatting",
            "registerOptions": {
                "documentSelector": [
                    {
                        "language": "vue"
                    }
                ]
            }
        }
    ]
}

[Trace - 11:34:26 AM] Received notification 'window/logMessage'.
Params: {
    "type": 3,
    "message": "Registering request handler for textDocument/formatting failed."
}


[Info  - 11:34:26 AM] Registering request handler for textDocument/formatting failed.
[Trace - 11:34:26 AM] Received notification 'window/logMessage'.
Params: {
    "type": 1,
    "message": "(node:72001) UnhandledPromiseRejectionWarning: Error: Unhandled method client/registerCapability\n    at handleResponse (/home/jeremy/Projects/libraries/vetur/server/node_modules/vscode-jsonrpc/lib/common/connection.js:476:48)\n    at processMessageQueue (/home/jeremy/Projects/libraries/vetur/server/node_modules/vscode-jsonrpc/lib/common/connection.js:291:17)\n    at Immediate.<anonymous> (/home/jeremy/Projects/libraries/vetur/server/node_modules/vscode-jsonrpc/lib/common/connection.js:275:13)\n    at processImmediate (internal/timers.js:439:21)"
}


[Error - 11:34:26 AM] (node:72001) UnhandledPromiseRejectionWarning: Error: Unhandled method client/registerCapability
    at handleResponse (/home/jeremy/Projects/libraries/vetur/server/node_modules/vscode-jsonrpc/lib/common/connection.js:476:48)
    at processMessageQueue (/home/jeremy/Projects/libraries/vetur/server/node_modules/vscode-jsonrpc/lib/common/connection.js:291:17)
    at Immediate.<anonymous> (/home/jeremy/Projects/libraries/vetur/server/node_modules/vscode-jsonrpc/lib/common/connection.js:275:13)
    at processImmediate (internal/timers.js:439:21)
[Trace - 11:34:26 AM] Received notification 'window/logMessage'.
Params: {
    "type": 1,
    "message": "(node:72001) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)"
}

rchl · 2020-11-06T11:35:13Z

Yes, I see that. Thanks for checking.

Will have a deeper look but as far as I can see the current code has two more problems:

registers capability too early (before initialize response)
tries to register capability multiple times

rwols · 2020-11-06T11:40:49Z

From what I understand, it's safe to do any request to the client after the client notified initialized: https://microsoft.github.io/language-server-protocol/specification#initialized

rchl · 2020-11-06T11:43:28Z

Right. And vetur sends client/registerCapability request before that. Before it even responds to intialize request (not to be confused with initialized notification).

rchl · 2020-11-06T13:31:19Z

@J3m5 latest changes should resolve the issue with double-registration and too-early-registration

J3m5 · 2020-11-06T13:38:12Z

The issue is gone, well done!

Per current version of the spec [1], the "documentSelector" is a list of DocumentFilter and can not contain a string. It seems like it was allowed to have a string in the past [2] but it's no longer the case and it doesn't work even in VSCode. [1] https://microsoft.github.io/language-server-protocol/specification#documentFilter [2] microsoft/vscode-languageserver-node#685 Resolves vuejs#2388

- setupDynamicFormatters() was called too early. Can only be called after `initialized`. - It should be enough to call it from "onDidChangeConfiguration" as that notification is guaranteed to be called by the client. - Avoid double-registration by not calling it from "init".

octref

LGTM, other than a small nitpick.

CHANGELOG.md

rchl mentioned this pull request Nov 5, 2020

Can a DocumentFilter also be a string? microsoft/language-server-protocol#1127

Closed

rchl force-pushed the fix/register-cap branch from c736932 to 23c636a Compare November 5, 2020 13:32

rchl mentioned this pull request Nov 6, 2020

Refactor to setupConfigure for #2388 #2389

Merged

rchl mentioned this pull request Nov 6, 2020

Bump vls from 0.5.4 to 0.5.5 in /server sublimelsp/LSP-vue#84

Merged

octref force-pushed the fix/register-cap branch from 923cb13 to 23c8373 Compare November 6, 2020 16:46

rchl added 2 commits November 7, 2020 00:47

octref approved these changes Nov 6, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

90194c3

octref merged commit 1efe1ef into vuejs:master Nov 6, 2020

rchl deleted the fix/register-cap branch November 6, 2020 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix invalid client/registerCapability request #2436

Fix invalid client/registerCapability request #2436

rchl commented Nov 5, 2020 •

edited

Loading

rchl commented Nov 5, 2020

yoyo930021 commented Nov 5, 2020 •

edited

Loading

rchl commented Nov 5, 2020 •

edited

Loading

J3m5 commented Nov 6, 2020

rchl commented Nov 6, 2020

rwols commented Nov 6, 2020

rchl commented Nov 6, 2020 •

edited

Loading

rchl commented Nov 6, 2020

J3m5 commented Nov 6, 2020

octref left a comment

Fix invalid client/registerCapability request #2436

Fix invalid client/registerCapability request #2436

Conversation

rchl commented Nov 5, 2020 • edited Loading

rchl commented Nov 5, 2020

yoyo930021 commented Nov 5, 2020 • edited Loading

rchl commented Nov 5, 2020 • edited Loading

J3m5 commented Nov 6, 2020

rchl commented Nov 6, 2020

rwols commented Nov 6, 2020

rchl commented Nov 6, 2020 • edited Loading

rchl commented Nov 6, 2020

J3m5 commented Nov 6, 2020

octref left a comment

Choose a reason for hiding this comment

rchl commented Nov 5, 2020 •

edited

Loading

yoyo930021 commented Nov 5, 2020 •

edited

Loading

rchl commented Nov 5, 2020 •

edited

Loading

rchl commented Nov 6, 2020 •

edited

Loading