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: don't access the network for go mod tidy diagnostics #38462

Closed
stamblerre opened this issue Apr 15, 2020 · 6 comments
Closed

x/tools/gopls: don't access the network for go mod tidy diagnostics #38462

stamblerre opened this issue Apr 15, 2020 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@stamblerre
Copy link
Contributor

A few reports of issues with gopls when the user has no internet (one on Slack, one on microsoft/vscode-go#3105).

I wonder if there is a way we can have a regression test this - maybe by configuring something in the environment? @findleyr

@stamblerre stamblerre added this to the gopls/v0.5.0 milestone Apr 15, 2020
@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 Apr 15, 2020
@findleyr
Copy link
Contributor

Following https://golang.org/cl/228235, the regtests use a proxy within our control. If we have examples of how gopls breaks without a network connection, we should be able to reproduce.

@mingoal
Copy link

mingoal commented Apr 15, 2020

@findleyr
suggest scenario:
in go file, include one package which has no local cache,

  1. without network connection, will gopls hang or not?
  2. with network connection which requires http proxy, will gopls hang or not?

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@findleyr findleyr self-assigned this Sep 3, 2020
@stamblerre stamblerre added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 25, 2020
@stamblerre stamblerre assigned heschi and unassigned findleyr Oct 29, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default Nov 10, 2020
@stamblerre stamblerre moved this from Needs Triage to In progress in vscode-go: gopls by default Nov 10, 2020
@stamblerre stamblerre moved this from In progress to Non-critical in vscode-go: gopls by default Nov 18, 2020
@stamblerre stamblerre moved this from Non-critical to Critical in vscode-go: gopls by default Nov 18, 2020
@gopherbot
Copy link

Change https://golang.org/cl/274120 mentions this issue: internal/lsp: disable network access for some go commands

gopherbot pushed a commit to golang/tools that referenced this issue Dec 3, 2020
For users with poor network connections, go command invocations that
access the network may hang up for long periods of time. Even for
users with good network connections, accessing proxy.golang.org or
github can take a few seconds, which is a long time to lock up an
editor.

Disable network access via GOPROXY=off when running most go commands.
There are two notable exceptions. First, the initial workspace load is
allowed, though subsequent loads are not. My reasoning is that if the
user is opening a project they've just downloaded, they probably expect
to fetch its dependencies. (Also, it's convenient to keep the regtests
going.) The second is the go mod tidy diagnostics, which I hope to
address in a later change. All the other commands that access the
network are at the user's request.

When the workspace load fails due to a dependency that hasn't been
downloaded, we present a quick fix on the go.mod line to do so. The only
way that happens is if there's a manual modification to the go.mod file,
since all of the other quick fixes will do the download. So I don't
think there's a need to present the fix anywhere else.

Updates golang/go#38462.

Change-Id: I470bc1ba790db7c1afee54d0b28fa0c6fd203fb9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274120
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@stamblerre stamblerre moved this from Critical to In progress in vscode-go: gopls by default Dec 6, 2020
@stamblerre
Copy link
Contributor Author

I believe the only remaining task here is to remove network access for the go mod tidy diagnostics.

@heschik, is there anything else I'm missing?

@heschi
Copy link
Contributor

heschi commented Dec 8, 2020

That's it as far as I know.

@stamblerre stamblerre changed the title x/tools/gopls: make sure gopls works without network connection x/tools/gopls: don't access the network for go mod tidy diagnostics Dec 8, 2020
@stamblerre stamblerre moved this from In progress to Non-critical in vscode-go: gopls by default Dec 8, 2020
@stamblerre stamblerre removed this from Non-critical in vscode-go: gopls by default Dec 16, 2020
marwan-at-work pushed a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
For users with poor network connections, go command invocations that
access the network may hang up for long periods of time. Even for
users with good network connections, accessing proxy.golang.org or
github can take a few seconds, which is a long time to lock up an
editor.

Disable network access via GOPROXY=off when running most go commands.
There are two notable exceptions. First, the initial workspace load is
allowed, though subsequent loads are not. My reasoning is that if the
user is opening a project they've just downloaded, they probably expect
to fetch its dependencies. (Also, it's convenient to keep the regtests
going.) The second is the go mod tidy diagnostics, which I hope to
address in a later change. All the other commands that access the
network are at the user's request.

When the workspace load fails due to a dependency that hasn't been
downloaded, we present a quick fix on the go.mod line to do so. The only
way that happens is if there's a manual modification to the go.mod file,
since all of the other quick fixes will do the download. So I don't
think there's a need to present the fix anywhere else.

Updates golang/go#38462.

Change-Id: I470bc1ba790db7c1afee54d0b28fa0c6fd203fb9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274120
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/289309 mentions this issue: internal/lsp/cache: disable network for mod tidy diagnostics

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.6.6 Feb 5, 2021
@golang golang locked and limited conversation to collaborators Feb 5, 2022
@rsc rsc unassigned heschi Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants