Skip to content

Commit 2d32e15

Browse files
committed
gopls/internal/lsp/cache: in tests, show all stacks during analyzer crash
We recently started using bug.Errorf to report analyzer crashes, causing tests to fail sporadically. Unfortunately the log included only the "panicked" message but not the relevant stack of the panic or of the other goroutines, giving limited evidence on which to investigate the underlying cause, suspected to be in package loading, not the analyzer itself. This change causes such crashes to display all stacks so that we can gather more evidence over the next few days before we suppress the crashes again. Updates golang/go#54762 Updates golang/go#56035 Change-Id: I2d29e05b5910540918816e695b458463a05f94d9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/439116 Reviewed-by: Bryan Mills <[email protected]> Run-TryBot: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent dc88e7b commit 2d32e15

File tree

1 file changed

+13
-12
lines changed

1 file changed

+13
-12
lines changed

gopls/internal/lsp/cache/analysis.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -361,19 +361,20 @@ func actionImpl(ctx context.Context, snapshot *snapshot, deps []*actionHandle, a
361361
var result interface{}
362362
var err error
363363
func() {
364-
// Set this flag temporarily when debugging crashes.
365-
// See https://github.com/golang/go/issues/54762.
366-
const norecover = false
367-
if norecover {
368-
debug.SetTraceback("all") // show all goroutines
369-
} else {
370-
defer func() {
371-
if r := recover(); r != nil {
372-
// Use bug.Errorf so that we detect panics during testing.
373-
err = bug.Errorf("analysis %s for package %s panicked: %v", analyzer.Name, pkg.PkgPath(), r)
364+
defer func() {
365+
if r := recover(); r != nil {
366+
// An Analyzer crashed. This is often merely a symptom
367+
// of a problem in package loading.
368+
if bug.PanicOnBugs {
369+
// During testing, crash. See issues 54762, 56035.
370+
debug.SetTraceback("all") // show all goroutines
371+
panic(r)
372+
} else {
373+
// In production, suppress the panic and press on.
374+
err = fmt.Errorf("analysis %s for package %s panicked: %v", analyzer.Name, pkg.PkgPath(), r)
374375
}
375-
}()
376-
}
376+
}
377+
}()
377378
result, err = pass.Analyzer.Run(pass)
378379
}()
379380
if err != nil {

0 commit comments

Comments
 (0)