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: reduce noisy log and error messages #66746

Closed
findleyr opened this issue Apr 9, 2024 · 3 comments
Closed

x/tools/gopls: reduce noisy log and error messages #66746

findleyr opened this issue Apr 9, 2024 · 3 comments
Assignees
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

@findleyr
Copy link
Contributor

findleyr commented Apr 9, 2024

This is an umbrella issue for work to reduce the number of noisy log and error messages produced by gopls.

Some of these have separate issues (e.g. #56395), but generally speaking gopls has grown far too chatty. In part, I think this has happened over time as Go calling semantics have leaked into LSP calling semantics. In a Go function, if I ask a for e.g. signatureHelp, and a function can't produce signatureHelp, it will typically return an error, perhaps with a sentinel value that indicates "there's no signature here". But in many LSP clients (including VS Code), all RPC errors are included in the server output.

In the past, I think errors have been suppressed in an ad-hoc manner, and we don't have a lot of tests that operations don't return an error.

We should enforce the semantics that an expected failure to produce a result is not an error. This issue tracks:

  • doing another pass through the code to eliminate noisy errors
  • adding tests for the non-existence of errors
  • also reducing superfluous log messages, or guarding them behind -verbose

CC @adonovan

@findleyr findleyr added this to the gopls/v0.16.0 milestone Apr 9, 2024
@findleyr findleyr self-assigned this Apr 9, 2024
@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 9, 2024
@gopherbot
Copy link

Change https://go.dev/cl/577655 mentions this issue: gopls: reduce noisy error messages

@gopherbot
Copy link

Change https://go.dev/cl/577875 mentions this issue: gopls/internal/cache: don't log packages when selectively reloading

gopherbot pushed a commit to golang/tools that referenced this issue Apr 10, 2024
- Fix a bug in the logic to suppress a context cancellation error
  computing the shared import graph.
- Remove ErrIsDefinition. It was introduced for testing in CL 196560,
  and is no longer needed.
- Update golang.SignatureHelp to return (nil, nil) in the case where
  SignatureHelp is simply not available.
- Suppress the mod tidy diagnostic error when GOPROXY=off. It is too
  noisy, and golang/go#56395 tracks the fix for the root cause.
- Suppress noisy errors about semantic tokens. Researching the history
  behind this error, it seems it was only added to force the
  invalidation of semantic tokens client-side. The same can be achieved
  by returning a non-nil, empty result.
- Enforce that marker tests encounter no error logs, with an -errors_ok
  flag to allow them in select cases.
- Update the -min_go version of some marker tests that use a go.mod file
  with a too-new go version. This was an error log.

For golang/go#66746

Change-Id: I21fe274e320cf5fa7d6b85d7402330fb647dbd29
Reviewed-on: https://go-review.googlesource.com/c/tools/+/577655
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit to golang/tools that referenced this issue Apr 10, 2024
Previously, gopls would log package contents whenever reloading (but not
reinitializing) a workspace--essentially whenever the query did not
contain a '...'. But following a big invalidation, gopls may reload
hundreds or even thousands of packages. Logging them is tremendously
verbose.

Change the behavior to simply log packages if and only if verboseOutput
is set. This is easier to understand and greatly reduces the noisiness
of loading.

Also annotate the one test that broke when I experimented with this
behavior. We should really have a more robust framework for asserting on
loads, but that is a project for another CL.

For golang/go#66746

Change-Id: I0eff7fad1b2deb2f170a1c336abf2c2a9e4f56ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/577875
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/579335 mentions this issue: gopls: normalize logging attributes

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

2 participants