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

LSP Headless Launch failing #512

Closed
chrisl8 opened this issue Oct 14, 2023 · 8 comments · Fixed by #513
Closed

LSP Headless Launch failing #512

chrisl8 opened this issue Oct 14, 2023 · 8 comments · Fixed by #513
Labels

Comments

@chrisl8
Copy link

chrisl8 commented Oct 14, 2023

Godot version

v4.2.beta1.official [b1371806a]

VS Code version

1.83.1

Godot Tools VS Code extension version

Direct from Master branch at commit d3b2c52

System information

Windows 11

Issue description

The error looks like this:
Screenshot 2023-10-13 234939

Before the error it says "Initializing LSP" for a while. Clicking on Retry will cycle through this gain, saying "Initializing LSP" again for a while then the error again, but with a new port each time.

Here are my settings:
Settings-01
Settings-02

The path to the Godot binary is:
C:\Dev\GodotEngines\Godot_v4.2-beta1_win64.exe\Godot_v4.2-beta1_win64.exe

This path works if I put it into the Win+r run box. I know it has .exe in it twice, but the Godot binary for Windows extracts into a folder named .exe, so that is how I use it.

Steps to reproduce

pull current master branch from github
npm i
npm run package

Install extension from vsix file

Set extension settings to run LSP headless.

@chrisl8 chrisl8 added the bug label Oct 14, 2023
@DaelonSuzuka
Copy link
Collaborator

Yeah that's behaving badly, alright. The fact that is says "initializing LSP" means thats it's attempting to start headless mode.

Your godot path is valid and executable and your godot binary responds with the correct version string, because it specifically checks each of those things and errors if fails...

Just for fun, can you change editorpath.godot4 to something you know is wrong, and then restart vscode?

@chrisl8
Copy link
Author

chrisl8 commented Oct 14, 2023

Just for fun, can you change editorpath.godot4 to something you know is wrong, and then restart vscode?

image

@DaelonSuzuka
Copy link
Collaborator

Okay, that means the checks are working correctly.

I just built and installed master and it's working for me on Windows 10. I'm not aware of any 10 -> 11 changes that should affect spawning a child process...

It looks to me like you have everything configured correctly, so I don't see why this would be failing. I have some more changes in progress that will touch the LSP client, so it's possible that the behavior will change as I finish those.

I can also look at the extension's logging and find a way to send that somewhere visible when in release mode.

@chrisl8
Copy link
Author

chrisl8 commented Oct 14, 2023

I opened up the extension in VSCode and ran it in "debug mode" (I've never done this before so I'm learning as I go) and that gave me a lot more useful output, and also helped me see what exact command line it was running.

It works on a blank project, so that told me my project was the problem, but it isn't an unusually fancy project.

What I saw is that my project spits out a lot of warnings when it starts, mostly warnings that if I save meshes they won't be backward compatible. It works though, unless started by VSCode in which case it hangs.

I uncommented these lines:
https://github.com/godotengine/godot-vscode-plugin/blob/d3b2c5227c98192fc89a72e06c9637c9e39f524a/src/lsp/ClientConnectionManager.ts#L164C1-L170C9

		// const lspStderr = createLogger("lsp.stderr");
		// lspProcess.stderr.on('data', (data) => {
		// 	const out = data.toString().trim();
		// 	if (out) {
		// 		lspStderr.debug(out);
		// 	}
		// });

in an attempt to get better visibility into what VSCode was seeing, but then it worked!

Just printing all of those stderr lines instead of ignoring them caused it to work fine.

I went ahead and built a .vsix file with these lines uncommented and installed it and now it works fine.

@chrisl8
Copy link
Author

chrisl8 commented Oct 14, 2023

My theory is that a buffer is filling up when there is a lot of stderr output and it is ignored, see:

https://stackoverflow.com/a/52863590/4982408
and
https://stackoverflow.com/a/20792428/4982408

@DaelonSuzuka
Copy link
Collaborator

Wow, what a crazy catch! I had commented that out during dev because it was too noisy. I planned to come back and add monitoring to the child process so the LSP could be restarted if it crashed, and I figured I could wait until then to re-enable stderr. I didn't expect that would sink the whole ship!

I'll get this fixed and merged right away.

I've never done this before so I'm learning as I go

You and me both I won't tell anyone if you don't.

@DaelonSuzuka
Copy link
Collaborator

DaelonSuzuka commented Oct 14, 2023

Allegedly fixed in #513.

@chrisl8 If you could grab the CI artifact from here (or checkout the PR, whichever) and confirm that it fixes the issue, I'd appreciate that.

@chrisl8
Copy link
Author

chrisl8 commented Oct 15, 2023

I tested it and yes, this appears to solve the issue. VSCode consistently starts and the LSP and uses it with this new CI artifact.

Thanks for the quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants