godotengine / godot-vscode-plugin

Godot development tools for VSCode
MIT License
1.52k stars 149 forks source link

LSP Headless Launch failing #512

Closed chrisl8 closed 10 months ago

chrisl8 commented 10 months ago

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 d3b2c5227c98192fc89a72e06c9637c9e39f524a

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.

DaelonSuzuka commented 10 months ago

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 commented 10 months ago

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

image

DaelonSuzuka commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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!