gopls/internal/lsp/cache: don't panic when import fails during analysis
We have received a number of empty crash reports for gopls since cutting
the v0.12.3 release. We don't want to auto-populate crash reports with
log.Fatal messages (as unlike panicking stacks they may contain the name
of user packages), so in almost all cases we lose information as to why
gopls crashed (users are unlikely to pre-populate this information).
In general, I don't think failing analysis should crash the process, so
switch this failure mode to a bug report.
Fixes golang/go#60963
Change-Id: I670ee4b1adbe9f37d763c5684b14d4bcb78dcb63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/505575
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit fa103593d02f86f400531e4294b9b294d4893901)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/505576
diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go
index ad55f2f..52e730b 100644
--- a/gopls/internal/lsp/cache/analysis.go
+++ b/gopls/internal/lsp/cache/analysis.go
@@ -408,8 +408,10 @@
exportDeps map[PackagePath]*analysisNode // subset of allDeps ref'd by export data (+self)
count int32 // number of unfinished successors
summary *analyzeSummary // serializable result of analyzing this package
- typesOnce sync.Once // guards lazy population of types field
- types *types.Package // type information lazily imported from summary
+
+ typesOnce sync.Once // guards lazy population of types and typesErr fields
+ types *types.Package // type information lazily imported from summary
+ typesErr error // an error producing type information
}
func (an *analysisNode) String() string { return string(an.m.ID) }
@@ -417,7 +419,7 @@
// _import imports this node's types.Package from export data, if not already done.
// Precondition: analysis was a success.
// Postcondition: an.types and an.exportDeps are populated.
-func (an *analysisNode) _import() *types.Package {
+func (an *analysisNode) _import() (*types.Package, error) {
an.typesOnce.Do(func() {
if an.m.PkgPath == "unsafe" {
an.types = types.Unsafe
@@ -439,12 +441,19 @@
}
an.exportDeps[path] = dep // record, for later fact decoding
if dep == an {
- items[i].Pkg = an.types
+ if an.typesErr != nil {
+ return an.typesErr
+ } else {
+ items[i].Pkg = an.types
+ }
} else {
i := i
g.Go(func() error {
- items[i].Pkg = dep._import()
- return nil
+ depPkg, err := dep._import()
+ if err == nil {
+ items[i].Pkg = depPkg
+ }
+ return err
})
}
}
@@ -452,13 +461,13 @@
}
pkg, err := gcimporter.IImportShallow(an.fset, getPackages, an.summary.Export, string(an.m.PkgPath))
if err != nil {
- log.Fatalf("%s: invalid export data: %v", an.m, err)
- }
- if pkg != an.types {
+ an.typesErr = bug.Errorf("%s: invalid export data: %v", an.m, err)
+ an.types = nil
+ } else if pkg != an.types {
log.Fatalf("%s: inconsistent packages", an.m)
}
})
- return an.types
+ return an.types, an.typesErr
}
// analyzeSummary is a gob-serializable summary of successfully
@@ -703,7 +712,10 @@
// Does the fact relate to a package referenced by export data?
if dep, ok := allExportDeps[PackagePath(path)]; ok {
dep.typesOnce.Do(func() { log.Fatal("dep.types not populated") })
- return dep.types
+ if dep.typesErr == nil {
+ return dep.types
+ }
+ return nil
}
// If the fact relates to a dependency not referenced
@@ -867,7 +879,7 @@
return nil, fmt.Errorf("invalid use of internal package %s", importPath)
}
- return dep._import(), nil
+ return dep._import()
}),
}
