internal/lsp: propagate and handle context cancellation errors
We don't distinguish between genuine errors and context cancellation in
diagnostics, which often results in superfluous logging of these errors.
Avoid spamming the logs with them by checking.
Also, remove the logic for sending undelivered diagnostics. It's a relic
of old bugs and isn't useful.
Change-Id: I7df226539b9b1eb49ab3aae8d7b0a67f59fb3058
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index ff50289..3e31b6f 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -38,8 +38,7 @@
for _, ah := range roots {
diagnostics, _, err := ah.analyze(ctx)
if err != nil {
- log.Error(ctx, "no results", err)
- continue
+ return nil, err
}
results = append(results, diagnostics...)
}
@@ -148,7 +147,7 @@
func (act *actionHandle) analyze(ctx context.Context) ([]*source.Error, interface{}, error) {
v := act.handle.Get(ctx)
if v == nil {
- return nil, nil, errors.Errorf("no analyses for %s", act.pkg.ID())
+ return nil, nil, ctx.Err()
}
data, ok := v.(*actionData)
if !ok {
@@ -160,21 +159,6 @@
return data.diagnostics, data.result, data.err
}
-func (act *actionHandle) cached() ([]*source.Error, interface{}, error) {
- v := act.handle.Cached()
- if v == nil {
- return nil, nil, errors.Errorf("no cached analyses for %s", act.pkg.ID())
- }
- data, ok := v.(*actionData)
- if !ok {
- return nil, nil, errors.Errorf("unexpected type for cached analysis %s:%s", act.pkg.ID(), act.analyzer.Name)
- }
- if data == nil {
- return nil, nil, errors.Errorf("unexpected nil cached analysis for %s:%s", act.pkg.ID(), act.analyzer.Name)
- }
- return data.diagnostics, data.result, data.err
-}
-
func buildActionKey(a *analysis.Analyzer, ph *packageHandle) string {
return hashContents([]byte(fmt.Sprintf("%p %s", a, string(ph.key))))
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index af7b59b..40fff9d 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -50,7 +50,7 @@
}
}
-func (s *Server) diagnoseFile(snapshot source.Snapshot, uri span.URI) error {
+func (s *Server) diagnoseFile(snapshot source.Snapshot, uri span.URI) {
ctx := snapshot.View().BackgroundContext()
ctx, done := trace.StartSpan(ctx, "lsp:background-worker")
defer done()
@@ -59,57 +59,51 @@
f, err := snapshot.View().GetFile(ctx, uri)
if err != nil {
- return err
+ log.Error(ctx, "diagnoseFile: no file", err)
+ return
}
reports, warningMsg, err := source.Diagnostics(ctx, snapshot, f, true, snapshot.View().Options().DisabledAnalyses)
- if err != nil {
- return err
- }
+ // Check the warning message first.
if warningMsg != "" {
s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
Type: protocol.Info,
Message: warningMsg,
})
}
+ if err != nil {
+ if err != context.Canceled {
+ log.Error(ctx, "diagnoseFile: could not generate diagnostics", err)
+ }
+ return
+ }
// Publish empty diagnostics for files.
s.publishReports(ctx, reports, true)
- return nil
}
func (s *Server) publishReports(ctx context.Context, reports map[source.FileIdentity][]source.Diagnostic, publishEmpty bool) {
- undelivered := make(map[source.FileIdentity][]source.Diagnostic)
+ // Check for context cancellation before publishing diagnostics.
+ if ctx.Err() != nil {
+ return
+ }
+
for fileID, diagnostics := range reports {
+ // Don't deliver diagnostics if the context has already been canceled.
+ if ctx.Err() != nil {
+ break
+ }
// Don't publish empty diagnostics unless specified.
if len(diagnostics) == 0 && !publishEmpty {
continue
}
- if err := s.publishDiagnostics(ctx, fileID, diagnostics); err != nil {
- undelivered[fileID] = diagnostics
-
- log.Error(ctx, "failed to deliver diagnostic (will retry)", err, telemetry.File)
+ if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
+ Diagnostics: toProtocolDiagnostics(ctx, diagnostics),
+ URI: protocol.NewURI(fileID.URI),
+ Version: fileID.Version,
+ }); err != nil {
+ log.Error(ctx, "failed to deliver diagnostic", err, telemetry.File)
continue
}
- // In case we had old, undelivered diagnostics.
- delete(undelivered, fileID)
}
- // Any time we compute diagnostics, make sure to also send along any
- // undelivered ones (only for remaining URIs).
- for uri, diagnostics := range undelivered {
- if err := s.publishDiagnostics(ctx, uri, diagnostics); err != nil {
- log.Error(ctx, "failed to deliver diagnostic for (will not retry)", err, telemetry.File)
- }
-
- // If we fail to deliver the same diagnostics twice, just give up.
- delete(undelivered, uri)
- }
-}
-
-func (s *Server) publishDiagnostics(ctx context.Context, fileID source.FileIdentity, diagnostics []source.Diagnostic) error {
- return s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
- Diagnostics: toProtocolDiagnostics(ctx, diagnostics),
- URI: protocol.NewURI(fileID.URI),
- Version: fileID.Version,
- })
}
func toProtocolDiagnostics(ctx context.Context, diagnostics []source.Diagnostic) []protocol.Diagnostic {
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 9f2d7b7..d53249b 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -56,15 +56,13 @@
// not correctly configured. Report errors, if possible.
var warningMsg string
if len(ph.MissingDependencies()) > 0 {
- warningMsg, err = checkCommonErrors(ctx, snapshot.View(), f.URI())
- if err != nil {
+ if warningMsg, err = checkCommonErrors(ctx, snapshot.View(), f.URI()); err != nil {
log.Error(ctx, "error checking common errors", err, telemetry.File.Of(f.URI))
}
}
pkg, err := ph.Check(ctx)
if err != nil {
- log.Error(ctx, "no package for file", err)
- return singleDiagnostic(fh.Identity(), "%s is not part of a package", f.URI()), "", nil
+ return nil, "", err
}
// Prepare the reports we will send for the files in this package.
reports := make(map[FileIdentity][]Diagnostic)
@@ -82,6 +80,10 @@
if !diagnostics(ctx, snapshot, pkg, reports) && withAnalysis {
// If we don't have any list, parse, or type errors, run analyses.
if err := analyses(ctx, snapshot, ph, disabledAnalyses, reports); err != nil {
+ // Exit early if the context has been canceled.
+ if err == context.Canceled {
+ return nil, "", err
+ }
log.Error(ctx, "failed to run analyses", err, telemetry.File.Of(f.URI()))
}
}
diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go
index d708bd0..30c243e 100644
--- a/internal/lsp/watched_files.go
+++ b/internal/lsp/watched_files.go
@@ -72,9 +72,10 @@
// If this was the only file in the package, clear its diagnostics.
if otherFile == nil {
- if err := s.publishDiagnostics(ctx, source.FileIdentity{
- URI: uri,
- }, []source.Diagnostic{}); err != nil {
+ if err := s.client.PublishDiagnostics(ctx, &protocol.PublishDiagnosticsParams{
+ URI: protocol.NewURI(uri),
+ Version: fh.Identity().Version,
+ }); err != nil {
log.Error(ctx, "failed to clear diagnostics", err, telemetry.URI.Of(uri))
}
return nil
