internal/lsp/cache: don't report a context error if load succeeded
If a context was canceled during load, only report it as a critical
error if the load actually failed.
Along the way, simplify evaulation of the critical error to use a switch
statement.
Also await IWL in the second Env used in shared regtests. Presumably it
is this Env that is being shutdown prior to IWL, triggering the
panicking code-path from golang/go#47030. I wasn't able to reproduce,
but all panics are occurring in regtest/misc, and this seems highly
plausible.
Fixes golang/go#47030
Change-Id: I4df65697f644cff275ab1babb783868fd9e10c2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/332589
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/internal/regtest/misc/shared_test.go b/gopls/internal/regtest/misc/shared_test.go
index 129a5ff..e6dc0ac 100644
--- a/gopls/internal/regtest/misc/shared_test.go
+++ b/gopls/internal/regtest/misc/shared_test.go
@@ -31,6 +31,7 @@
// Create a second test session connected to the same workspace and server
// as the first.
env2 := NewEnv(env1.Ctx, t, env1.Sandbox, env1.Server, env1.Editor.Config, true)
+ env2.Await(InitialWorkspaceLoad)
testFunc(env1, env2)
})
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index cb08ea0..3f39882 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -607,35 +607,40 @@
} else {
scopes = append(scopes, viewLoadScope("LOAD_VIEW"))
}
- var err error
+
+ // If we're loading anything, ensure we also load builtin.
+ // TODO(rstambler): explain the rationale for this.
if len(scopes) > 0 {
- err = s.load(ctx, firstAttempt, append(scopes, packagePath("builtin"))...)
+ scopes = append(scopes, packagePath("builtin"))
}
+ err := s.load(ctx, firstAttempt, scopes...)
+
// If the context is canceled on the first attempt, loading has failed
// because the go command has timed out--that should be a critical error.
- if !firstAttempt && ctx.Err() != nil {
+ if err != nil && !firstAttempt && ctx.Err() != nil {
return
}
var criticalErr *source.CriticalError
- if ctx.Err() != nil {
+ switch {
+ case err != nil && ctx.Err() != nil:
event.Error(ctx, fmt.Sprintf("initial workspace load: %v", err), err)
criticalErr = &source.CriticalError{
MainError: err,
}
- } else if err != nil {
+ case err != nil:
event.Error(ctx, "initial workspace load failed", err)
extractedDiags, _ := s.extractGoCommandErrors(ctx, err.Error())
criticalErr = &source.CriticalError{
MainError: err,
DiagList: append(modDiagnostics, extractedDiags...),
}
- } else if len(modDiagnostics) == 1 {
+ case len(modDiagnostics) == 1:
criticalErr = &source.CriticalError{
MainError: fmt.Errorf(modDiagnostics[0].Message),
DiagList: modDiagnostics,
}
- } else if len(modDiagnostics) > 1 {
+ case len(modDiagnostics) > 1:
criticalErr = &source.CriticalError{
MainError: fmt.Errorf("error loading module names"),
DiagList: modDiagnostics,
