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

x/tools/gopls: -remote causes the first run with Helix to not work for a few seconds #70298

Open
mvdan opened this issue Nov 12, 2024 · 10 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Nov 12, 2024

What did you do?

Running gopls with https://helix-editor.com/ and with -remote=auto, I open a Go file inside a Go module for the first time without gopls already running in remote mode, and ask gopls a question very quickly, such as go-to-definition or the workspace symbol picker.

What did you expect to see?

It should work, even if it takes a few seconds to give an answer due to the LSP loading the module.

What did you see instead?

An error from Helix:

No configured language server supports workspace symbols

This error seems to come from the editor, so it may not be a gopls bug. However, if I remove -remote=auto as suggested by @alandonovan, then the bug disappears - I can kill gopls, open a file and very quickly list all workspace symbols, and the UI pops up immediately without a problem, even if it takes a couple of seconds for the symbols to show up.

Presumably an editor like Helix starting the LSP causes a handshake to happen for the editor to know which features the LSP supports, so I wonder if this doesn't work too well when -remote=auto is used, causing the editor to think a Go LSP is not supported or present while it is still loading.

Build info

golang.org/x/tools/gopls v0.0.0-20241111214603-06a498a7a312
    golang.org/x/tools/gopls@v0.0.0-20241111214603-06a498a7a312 h1:7KWJGDbH0ZlvryN3TlO+JzxJWieJHrk0mPS20KRc8uE=
    github.com/BurntSushi/toml@v1.4.1-0.20240526193622-a339e1f7089c h1:pxW6RcqyfI9/kWtOwnv/G+AzdKuy2ZrqINhenH4HyNs=
    github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
    golang.org/x/exp/typeparams@v0.0.0-20231108232855-2478ac86f678 h1:1P7xPZEwZMoBoz0Yze5Nx2/4pxj6nw9ZqHWXqP0iRgQ=
    golang.org/x/mod@v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
    golang.org/x/sync@v0.9.0 h1:fEo0HyrW1GIgZdpbhCRO0PkJajUS5H9IFUztCgEo2jQ=
    golang.org/x/telemetry@v0.0.0-20241106142447-58a1122356f5 h1:TCDqnvbBsFapViksHcHySl/sW4+rTGNIAoJJesHRuMM=
    golang.org/x/text@v0.20.0 h1:gK/Kv2otX8gz+wn7Rmb3vT96ZwuoxnQlY+HlJVj7Qug=
    golang.org/x/tools@v0.27.1-0.20241111214603-06a498a7a312 h1:8k/Q1o+SUyt5050kQIaxFUZdu3rDC6XHaiASBndZKn0=
    golang.org/x/vuln@v1.0.4 h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
    honnef.co/go/tools@v0.5.1 h1:4bH5o3b5ZULQ4UrBmP+63W9r7qIkqJClEA9ko5YKx+I=
    mvdan.cc/gofumpt@v0.7.0 h1:bg91ttqXmi9y2xawvkuMXyvAA/1ZGJqYAEGjXuP0JXU=
    mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: devel go1.24-c96939fbed 2024-11-12 01:08:33 +0000
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Nov 12, 2024
@gopherbot gopherbot added this to the Unreleased milestone Nov 12, 2024
@mvdan
Copy link
Member Author

mvdan commented Nov 12, 2024

FYI @findleyr I'm happy to share screen recordings or do a brief video call if you would like me to show this behavior, get you more detailed logs, or try different things. For now, as Alan suggested, I no longer use -remote=auto because gopls is efficient enough these days where I don't have to keep it alive in the background to ensure my editor starts quickly.

@findleyr
Copy link
Member

@mvdan I think in general we do not need to advertise -remote because we've instead allowed gopls to share state via a filesystem cache, so I don't think this is a high priority to fix.

I'd also like to prioritize switching to jsonrpc2_v2 over this, since it may affect the symptoms (I almost got it working in my idle time a couple weeks ago: #52838 (comment)).

@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Nov 13, 2024
@findleyr
Copy link
Member

Thank you for the report though: -remote should not affect anything from Helix's perspective, so this is clearly a bug.

@findleyr
Copy link
Member

One further quick thought: I might try running gopls in a terminal with -listen=localhost:8091 and adding -remote=localhost:8091 to helix. I would be interested in knowing two things:

  • Does the symptom persist?
  • Does your gopls daemon crash? It may be that there is a crash that you don't notice due to helix restarting the LSP client.

@mvdan
Copy link
Member Author

mvdan commented Nov 14, 2024

Yes, my use of -remote was from years ago, from a time when gopls wasn't as efficient as it is today. Alan thankfully pointed that out. I agree this is not high priority, and perhaps we should even deprecate and consider removing -remote=auto because I think any existing users should move away from it.

I'll try your suggestions and report back.

@mvdan
Copy link
Member Author

mvdan commented Nov 14, 2024

With -remote=localhost:8091, the issue does NOT persist. The logs also look clean on the server, and I don't see any error or crash:

$ gopls -listen=localhost:8091
serve.go:132: Gopls daemon: listening on tcp network, address localhost:8091...
lsprpc.go:90: Session 1: connected
lsprpc.go:472: Session 1: got handshake. Logfile: "", Debug addr: ""
lsprpc.go:94: Session 1: exited

@mvdan
Copy link
Member Author

mvdan commented Nov 14, 2024

I should note that this is perhaps different from -remote=auto in that I started the server, and then it took me a solid five seconds until I started the editor. If -remote=auto does those two one immediately after the other, perhaps there is some sort of race condition happening.

@findleyr
Copy link
Member

consider removing -remote=auto

I mostly agree with removing -remote=auto, though the -remote flag remains extremely useful for debugging.

Yes, it sounds like the problem may be in the initial connection to the auto-started daemon.

@mvdan
Copy link
Member Author

mvdan commented Nov 14, 2024

If we are deprecating or removing -remote=auto then I'm perfectly fine to close this as won't fix - and perhaps one more reason why it should go away. I definitely won't use this mode again, so I don't care that the bug exists at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants