internal/lsp: avoid logging context cancellation
This change adds a helper function that checks if the context is
canceled, and if so, doesn't log the error. Tried to use it everywhere
in internal/lsp where it fits, which resulted in changing a few pieces
of error handling throughout.
Updates golang/go#37875
Change-Id: I59cbc6f893e3b70cf84524d9944ff7f4b4febd78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226371
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index a81a0b8..ecedc31 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -348,6 +348,10 @@
event.Error(ctx, "unable to compute analysis error position", err, tag.Category.Of(diag.Category), tag.Package.Of(pkg.ID()))
continue
}
+ if ctx.Err() != nil {
+ data.err = ctx.Err()
+ return data
+ }
data.diagnostics = append(data.diagnostics, srcErr)
}
return data
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 2e0cc5e..09e2950 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -144,7 +144,9 @@
depHandle, err := s.buildPackageHandle(ctx, depID, mode)
if err != nil {
event.Error(ctx, "no dep handle", err, tag.Package.Of(string(depID)))
-
+ if ctx.Err() != nil {
+ return nil, nil, ctx.Err()
+ }
// One bad dependency should not prevent us from checking the entire package.
// Add a special key to mark a bad dependency.
depKeys = append(depKeys, packageHandleKey(fmt.Sprintf("%s import not found", id)))
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index 4ea0cf0..966b5a9 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -60,6 +60,9 @@
kind = source.ParseError
spn, err = scannerErrorRange(fset, pkg, e.Pos)
if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
event.Error(ctx, "no span for scanner.Error pos", err, tag.Package.Of(pkg.ID()))
spn = span.Parse(e.Pos.String())
}
@@ -73,6 +76,9 @@
kind = source.ParseError
spn, err = scannerErrorRange(fset, pkg, e[0].Pos)
if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
event.Error(ctx, "no span for scanner.Error pos", err, tag.Package.Of(pkg.ID()))
spn = span.Parse(e[0].Pos.String())
}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index 75a2cae..51dac8a 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -16,7 +16,6 @@
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/telemetry/event"
- errors "golang.org/x/xerrors"
)
func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) {
@@ -44,7 +43,7 @@
}
}
if len(wanted) == 0 {
- return nil, errors.Errorf("no supported code action to execute for %s, wanted %v", uri, params.Context.Only)
+ return nil, fmt.Errorf("no supported code action to execute for %s, wanted %v", uri, params.Context.Only)
}
var codeActions []protocol.CodeAction
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index cd94bbf..edc2b08 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -28,9 +28,8 @@
case source.Mod:
candidates, surrounding = nil, nil
}
-
if err != nil {
- event.Print(ctx, "no completions found", tag.Position.Of(params.Position), event.Err.Of(err))
+ event.Error(ctx, "no completions found", err, tag.Position.Of(params.Position))
}
if candidates == nil {
return &protocol.CompletionList{
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index d0f1711..b98d901 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -57,14 +57,14 @@
// Diagnose the go.mod file.
reports, missingModules, err := mod.Diagnostics(ctx, snapshot)
- if ctx.Err() != nil {
- return nil
- }
if err != nil {
event.Error(ctx, "warning: diagnose go.mod", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
}
- // Ensure that the reports returned from mod.Diagnostics are only related to the
- // go.mod file for the module.
+ if ctx.Err() != nil {
+ return nil
+ }
+ // Ensure that the reports returned from mod.Diagnostics are only related
+ // to the go.mod file for the module.
if len(reports) > 1 {
panic("unexpected reports from mod.Diagnostics")
}
@@ -81,9 +81,6 @@
// Diagnose all of the packages in the workspace.
wsPackages, err := snapshot.WorkspacePackages(ctx)
- if ctx.Err() != nil {
- return nil
- }
if err != nil {
event.Error(ctx, "failed to load workspace packages, skipping diagnostics", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder))
return nil
@@ -107,9 +104,6 @@
Message: `You are neither in a module nor in your GOPATH. If you are using modules, please open your editor at the directory containing the go.mod. If you believe this warning is incorrect, please file an issue: https://github.com/golang/go/issues/new.`,
})
}
- if ctx.Err() != nil {
- return
- }
if err != nil {
event.Error(ctx, "warning: diagnose package", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
return
@@ -185,9 +179,7 @@
URI: protocol.URIFromSpanURI(key.id.URI),
Version: key.id.Version,
}); err != nil {
- if ctx.Err() == nil {
- event.Error(ctx, "publishReports: failed to deliver diagnostic", err, tag.URI.Of(key.id.URI))
- }
+ event.Error(ctx, "publishReports: failed to deliver diagnostic", err, tag.URI.Of(key.id.URI))
continue
}
// Update the delivered map.
diff --git a/internal/lsp/link.go b/internal/lsp/link.go
index d898df4..3a7fb6f 100644
--- a/internal/lsp/link.go
+++ b/internal/lsp/link.go
@@ -56,13 +56,13 @@
// dependency within the require statement.
start, end := token.Pos(s+i), token.Pos(s+i+len(dep))
target := fmt.Sprintf("https://%s/mod/%s", view.Options().LinkTarget, req.Mod.String())
- if l, err := toProtocolLink(view, m, target, start, end, source.Mod); err == nil {
- links = append(links, l)
- } else {
- event.Error(ctx, "failed to create protocol link", err)
+ l, err := toProtocolLink(view, m, target, start, end, source.Mod)
+ if err != nil {
+ return nil, err
}
+ links = append(links, l)
}
- // TODO(ridersofrohan): handle links for replace and exclude directives
+ // TODO(ridersofrohan): handle links for replace and exclude directives.
if syntax := file.Syntax; syntax == nil {
return links, nil
}
@@ -110,11 +110,12 @@
target = fmt.Sprintf("https://%s/%s", view.Options().LinkTarget, target)
// Account for the quotation marks in the positions.
start, end := n.Path.Pos()+1, n.Path.End()-1
- if l, err := toProtocolLink(view, m, target, start, end, source.Go); err == nil {
- links = append(links, l)
- } else {
- event.Error(ctx, "failed to create protocol link", err)
+ l, err := toProtocolLink(view, m, target, start, end, source.Go)
+ if err != nil {
+ event.Error(ctx, "failed to create link", err)
+ return false
}
+ links = append(links, l)
}
return false
case *ast.BasicLit:
diff --git a/internal/lsp/protocol/context.go b/internal/lsp/protocol/context.go
index 560de4d..dac850d 100644
--- a/internal/lsp/protocol/context.go
+++ b/internal/lsp/protocol/context.go
@@ -19,6 +19,9 @@
}
func LogEvent(ctx context.Context, ev event.Event, tags event.TagMap) context.Context {
+ if ctx.Err() != nil {
+ return ctx
+ }
if !ev.IsLog() {
return ctx
}
diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go
index 4e3afc8..e168534 100644
--- a/internal/lsp/source/completion_format.go
+++ b/internal/lsp/source/completion_format.go
@@ -247,9 +247,7 @@
item.Kind = protocol.FunctionCompletion
astObj, err := c.snapshot.View().LookupBuiltin(c.ctx, obj.Name())
if err != nil {
- if c.ctx.Err() == nil {
- event.Error(c.ctx, "no builtin package", err)
- }
+ event.Error(c.ctx, "no builtin package", err)
break
}
decl, ok := astObj.Decl.(*ast.FuncDecl)
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index dbf7529..b5b0444 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -131,10 +131,10 @@
analyzers = snapshot.View().Options().TypeErrorAnalyzers
}
if err := analyses(ctx, snapshot, reports, ph, analyzers); err != nil {
+ event.Error(ctx, "analyses failed", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
if ctx.Err() != nil {
return nil, warn, ctx.Err()
}
- event.Error(ctx, "failed to run analyses", err, tag.Package.Of(ph.ID()))
}
return reports, warn, nil
}
@@ -227,7 +227,6 @@
}
file, _, m, _, err := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseHeader).Parse(ctx)
if err != nil {
- event.Error(ctx, "could not parse go file when checking for missing modules", err)
return err
}
// Make a dependency->import map to improve performance when finding missing dependencies.
diff --git a/internal/lsp/source/highlight.go b/internal/lsp/source/highlight.go
index de4d6f8..5b92a26 100644
--- a/internal/lsp/source/highlight.go
+++ b/internal/lsp/source/highlight.go
@@ -40,7 +40,7 @@
}
path, _ := astutil.PathEnclosingInterval(file, rng.Start, rng.Start)
if len(path) == 0 {
- return nil, errors.Errorf("no enclosing position found for %v:%v", int(pos.Line), int(pos.Character))
+ return nil, fmt.Errorf("no enclosing position found for %v:%v", int(pos.Line), int(pos.Character))
}
// If start==end for astutil.PathEnclosingInterval, the 1-char interval following start is used instead.
// As a result, we might not get an exact match so we should check the 1-char interval to the left of the
@@ -147,10 +147,9 @@
if resultsList != nil && -1 < index && index < len(resultsList.List) {
rng, err := nodeToProtocolRange(view, pkg, resultsList.List[index])
if err != nil {
- event.Error(ctx, "Error getting range for node", err)
- } else {
- result[rng] = true
+ return nil, err
}
+ result[rng] = true
}
// Add the "func" part of the func declaration.
if highlightAllReturnsAndFunc {
@@ -185,9 +184,9 @@
rng, err := nodeToProtocolRange(view, pkg, toAdd)
if err != nil {
event.Error(ctx, "Error getting range for node", err)
- } else {
- result[rng] = true
+ return false
}
+ result[rng] = true
return false
}
}
@@ -268,11 +267,12 @@
if !strings.Contains(basicLit.Value, obj.Name()) {
return true
}
- if rng, err := nodeToProtocolRange(view, pkg, n); err == nil {
- result[rng] = true
- } else {
+ rng, err := nodeToProtocolRange(view, pkg, n)
+ if err != nil {
event.Error(ctx, "Error getting range for node", err)
+ return false
}
+ result[rng] = true
return false
})
return rangeMapToSlice(result), nil
@@ -311,11 +311,12 @@
if nObj := pkg.GetTypesInfo().ObjectOf(n); nObj != idObj {
return false
}
- if rng, err := nodeToProtocolRange(view, pkg, n); err == nil {
- result[rng] = true
- } else {
+ rng, err := nodeToProtocolRange(view, pkg, n)
+ if err != nil {
event.Error(ctx, "Error getting range for node", err)
+ return false
}
+ result[rng] = true
return false
})
return rangeMapToSlice(result), nil
