Skip to content

Commit

Permalink
gopls/internal/lsp/cache: don't panic when import fails during analysis
Browse files Browse the repository at this point in the history
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 fa10359)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/505576
  • Loading branch information
findleyr committed Jun 23, 2023
1 parent 72cd703 commit 1257328
Showing 1 changed file with 24 additions and 12 deletions.
36 changes: 24 additions & 12 deletions gopls/internal/lsp/cache/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,16 +408,18 @@ type analysisNode struct {
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) }

// _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
Expand All @@ -439,26 +441,33 @@ func (an *analysisNode) _import() *types.Package {
}
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
})
}
}
return g.Wait()
}
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
Expand Down Expand Up @@ -703,7 +712,10 @@ func (an *analysisNode) run(ctx context.Context) (*analyzeSummary, error) {
// 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
Expand Down Expand Up @@ -867,7 +879,7 @@ func (an *analysisNode) typeCheck(parsed []*source.ParsedGoFile) *analysisPackag
return nil, fmt.Errorf("invalid use of internal package %s", importPath)
}

return dep._import(), nil
return dep._import()
}),
}

Expand Down

0 comments on commit 1257328

Please sign in to comment.