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

Test: LSP Inspector JSON log output and streaming #57310

Closed
3 tasks done
octref opened this issue Aug 28, 2018 · 9 comments
Closed
3 tasks done

Test: LSP Inspector JSON log output and streaming #57310

octref opened this issue Aug 28, 2018 · 9 comments

Comments

@octref
Copy link
Contributor

octref commented Aug 28, 2018

Testing microsoft/language-server-protocol-inspector#8 and microsoft/language-server-protocol-inspector#5.

Complexity: 3

This PR comment explains the high level change: microsoft/vscode-languageserver-node#366 (comment)

JSON Log output: Language Servers that use https://github.com/Microsoft/vscode-languageserver-node have the ability to use [langId].trace.server option to direct logs to an Output Channel, see: https://code.visualstudio.com/docs/extensions/example-language-server#_logging-support-for-language-server

In the past you have the setting [langId].trace.server: "off" | "message" | "verbose". Now the format is:

[langId].trace.server: {
  verbosity: "off" | "message" | "verbose",
  "format": "text" | "json"
}

Test JSON log:

  • Use the next version of vscode-languageclient and vscode-languageserver
  • Modify https://github.com/Microsoft/vscode-extension-samples/tree/master/lsp-sample so that it uses the latest version of vscode-languageserver-node
  • Try the old [langId].trace.server options and make sure they still work.
  • Try the new option described above, and make sure both the text output and json options work as you would expect.

Test Streaming:

@bpasero bpasero added this to the August 2018 milestone Aug 28, 2018
@sandy081 sandy081 assigned chrmarti and unassigned aeschli Aug 28, 2018
@isidorn isidorn assigned dbaeumer and unassigned isidorn Aug 28, 2018
@bpasero
Copy link
Member

bpasero commented Aug 28, 2018

@octref @dbaeumer please update the test plan to make this test able. It seems to lack proper setup steps.

@bpasero bpasero added info-needed Issue requires more information from poster and removed info-needed Issue requires more information from poster labels Aug 28, 2018
@bpasero
Copy link
Member

bpasero commented Aug 28, 2018

Seems to work when I set "vscode-languageserver": "^5.0.3" in the server and configure:

"json.trace.server": {
    "verbosity": "verbose",
    "format": "json"
}

The warning in the settings editor is expected according to Dirk.

@bpasero
Copy link
Member

bpasero commented Aug 28, 2018

@octref I would appreciate if you could take a bit more time to make it possible/easier for a tester to test the second part (LSP inspector). Pointing to commits is not ideal, how would a real extension author discover how to do this? At the minimum I suggest to provide a sample in our vscode-extension-sample repository that leverages this functionality. Thanks!

@dbaeumer
Copy link
Member

dbaeumer commented Aug 28, 2018

The version to use should simply be "next" on the client and server

@dbaeumer
Copy link
Member

I couldn't get the streaming to work. I added the code and the following setting but in the extension I always got that no socket is open.

	"mls.debug.log": {
		"output": "websocket",
		"port": 9999
	}

I would highly recommend to consider https://github.com/octref/vscode-lsp-inspector/issues/1 since a socket transport looks very complicated to set up.

@dbaeumer dbaeumer removed their assignment Aug 28, 2018
@chrmarti
Copy link
Collaborator

The referenced commit does not work for me. It first complained about the connection not being open and after working around that it now fails parsing JSON in the webview. Not sure what schema it would expect. https://github.com/octref/vscode-lsp-inspector/issues/1 sounds like a good idea.

@octref
Copy link
Contributor Author

octref commented Aug 29, 2018

Pointing to commits is not ideal, how would a real extension author discover how to do this? At the minimum I suggest to provide a sample in our vscode-extension-sample repository that leverages this functionality. Thanks!

@bpasero Thanks for the feedback. I should have provided more detailed instructions and a sample. Also I shouldn't test "changing your code to add WebSocket support" without writing documentation first.

However, one issue is this page (https://code.visualstudio.com/docs/extensions/example-language-server) is already a 5000 words essay. I really want to avoid adding anything too specific to it, but meanwhile our extensibility doc is already a kitchen-sink and I want to avoid making it messier.

I'll open another issue to suggest improving organization of our extensibility doc.
Mean while I added samples: microsoft/vscode-extension-samples#88. See the last section of this comment for more details.


It seems you are all able to test the JSON output part, so here is an updated description for testing the LSP Inspector:

Preparation

Streaming

  • Make sure both extensions are installed.
  • Make sure you are using the correct config for server trace, e.g.
    "languageServerExample.trace.server": {
      "format": "json",
      "verbosity": "verbose"
    }
    
  • Run LSP Inspector: Start LSP Inspector first. This would open the LSP Inspector.
  • Open a plaintext file. This would activate the lsp-stream-sample extension.
  • Run Start Stream Logs into languageServerExample.port. This should start directing the logs into the LSP Inspector.

Check that

The Commits on branch json-streaming of vscode-extension-samples

@bpasero
Copy link
Member

bpasero commented Aug 29, 2018

@octref this is very cool, I verified it works on macOS. One question: wouldn't it make sense that the LSP library is providing this functionality out of the box instead of asking each extension to create and setup a websocket connection? I wonder if you need an extra library for this or if you could not just use node.js directly (to prevent the extra dependency)?

@octref
Copy link
Contributor Author

octref commented Aug 29, 2018

@bpasero There is no native WebSocket object in Node. However the ws module has a very shallow dependency tree, so it does not add too much to vscode-languageserver-node:

├─ ws@6.0.0
│  └─ async-limiter@~1.0.0

If we do end up adding WebSocket support built-in for the LS library, ws wouldn't be too much a problem.

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants