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

Crash on invoking function signature. #1045

Closed
hifall opened this issue Sep 1, 2018 · 5 comments
Closed

Crash on invoking function signature. #1045

hifall opened this issue Sep 1, 2018 · 5 comments

Comments

@hifall
Copy link

hifall commented Sep 1, 2018

monaco-editor version: 0.14.3
Browser: Chrome 67.0.3396.99
OS: macOS 10.12

Okay, it seems like a VS Code issue from where the crash happens, but since I am using Monaco, I am filing it here.

I am using Monaco editor connected with a backend language server. When trying to invoke displaying the signature of a function, I am seeing a crash with the following call stack in the browser console:

errors.js:58 Uncaught Error: Cannot read property 'length' of undefined

TypeError: Cannot read property 'length' of undefined
    at ParameterHintsWidget.render (parameterHintsWidget.js:272)
    at parameterHintsWidget.js:182
    at Emitter.fire (event.js:105)
    at parameterHintsWidget.js:104
    at ParameterHintsWidget.render (parameterHintsWidget.js:272)
    at parameterHintsWidget.js:182
    at Emitter.fire (event.js:105)
    at parameterHintsWidget.js:104
    at errors.js:58

The Monaco client sends a request like this:

{"jsonrpc":"2.0","id":6,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"/home/model.r"},"position":{"line":36,"character":6}}}

And the language server responds with:

{"jsonrpc":"2.0","id":6,"result":{"signatures":[{"label":"paste(..., sep = \" \", collapse = NULL)"}],"activeSignature":0}}

This happens because of the following line inside ParameterHintsWidget's render function:

var hasParameters = signature.parameters.length > 0;

This line assumes that parameters field always present inside signature, while in fact parameters is absent from the server response above. However, the LSP specification says SignatureInformation's parameters is optional. Therefore, the assumption doesn't seem correct.

@rcjsuen
Copy link
Contributor

rcjsuen commented Sep 1, 2018

@hifall How are you hooking up the language server to your Monaco editor? Are you using TypeFox's monaco-languageclient?

@hifall
Copy link
Author

hifall commented Sep 1, 2018

@rcjsuen, yes.

@rcjsuen
Copy link
Contributor

rcjsuen commented Sep 1, 2018

https://github.com/Microsoft/monaco-editor/blob/935bd5275a4e9170db586c514051936c0749237b/monaco.d.ts#L4763-L4783

@hifall As the API does not state that it is optional, I believe this is a bug in monaco-languageclient. See TypeFox/monaco-languageclient#112.

Of course, if the VS Code team wants to loosen up their API requirement, they are welcome to do so. Irregardless of their decision, we should fix monaco-languageclient in the meantime...

@hifall
Copy link
Author

hifall commented Sep 1, 2018

The official spec says it's optional: LSP spec, search for SignatureInformation.

@rcjsuen
Copy link
Contributor

rcjsuen commented Sep 1, 2018

@hifall That's the LSP specification and not the Monaco/VS Code API. They may look like they are 1-to-1 but they do not have to be 1-to-1.

It's up to bridges like the TypeFox/monaco-languageclient and Microsoft/vscode-languageserver-node projects to alter LSP information into a way that is consumable by the client editor.

@alexdima alexdima closed this as completed Sep 6, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants