From 7e1a0f1f161c65e0cab4efc410cc98c6605dad6b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 19 Mar 2023 19:21:35 +0100 Subject: [PATCH 1/3] fix: improve panic management --- pkg/golinters/goanalysis/runner_action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/golinters/goanalysis/runner_action.go b/pkg/golinters/goanalysis/runner_action.go index 40185a704070..d83a9a6ed7f3 100644 --- a/pkg/golinters/goanalysis/runner_action.go +++ b/pkg/golinters/goanalysis/runner_action.go @@ -98,6 +98,12 @@ func (act *action) waitUntilDependingAnalyzersWorked() { func (act *action) analyzeSafe() { defer func() { if p := recover(); p != nil { + // This line allows to display "hidden" panic with analyzers like buildssa. + // Some linters are dependent of sub-analyzers but when a sub-analyzer fails the linter is not aware of that, + // this results to another panic (ex: "interface conversion: interface {} is nil, not *buildssa.SSA"). + // This line will create a duplication of the panic message on purpose. + act.r.log.Errorf("panic during analysis: %v, %s", p, string(debug.Stack())) + act.err = errorutil.NewPanicError(fmt.Sprintf("%s: package %q (isInitialPkg: %t, needAnalyzeSource: %t): %s", act.a.Name, act.pkg.Name, act.isInitialPkg, act.needAnalyzeSource, p), debug.Stack()) } From f9e972604e781fff34b96cb14052f8f8df2e98d3 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Sun, 19 Mar 2023 21:58:31 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Anton Telyshev --- pkg/golinters/goanalysis/runner_action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/golinters/goanalysis/runner_action.go b/pkg/golinters/goanalysis/runner_action.go index d83a9a6ed7f3..2de80623f088 100644 --- a/pkg/golinters/goanalysis/runner_action.go +++ b/pkg/golinters/goanalysis/runner_action.go @@ -102,7 +102,7 @@ func (act *action) analyzeSafe() { // Some linters are dependent of sub-analyzers but when a sub-analyzer fails the linter is not aware of that, // this results to another panic (ex: "interface conversion: interface {} is nil, not *buildssa.SSA"). // This line will create a duplication of the panic message on purpose. - act.r.log.Errorf("panic during analysis: %v, %s", p, string(debug.Stack())) + act.r.log.Errorf("%s: panic during analysis: %v, %s", act.a.Name, p, string(debug.Stack())) act.err = errorutil.NewPanicError(fmt.Sprintf("%s: package %q (isInitialPkg: %t, needAnalyzeSource: %t): %s", act.a.Name, act.pkg.Name, act.isInitialPkg, act.needAnalyzeSource, p), debug.Stack()) From ea36e343b8fe468e34f88b2fca6448c60d57e160 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 19 Mar 2023 22:08:46 +0100 Subject: [PATCH 3/3] fix: log only if not root analyzer --- pkg/golinters/goanalysis/runner_action.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/golinters/goanalysis/runner_action.go b/pkg/golinters/goanalysis/runner_action.go index 2de80623f088..a9d50c4c84f4 100644 --- a/pkg/golinters/goanalysis/runner_action.go +++ b/pkg/golinters/goanalysis/runner_action.go @@ -98,11 +98,12 @@ func (act *action) waitUntilDependingAnalyzersWorked() { func (act *action) analyzeSafe() { defer func() { if p := recover(); p != nil { - // This line allows to display "hidden" panic with analyzers like buildssa. - // Some linters are dependent of sub-analyzers but when a sub-analyzer fails the linter is not aware of that, - // this results to another panic (ex: "interface conversion: interface {} is nil, not *buildssa.SSA"). - // This line will create a duplication of the panic message on purpose. - act.r.log.Errorf("%s: panic during analysis: %v, %s", act.a.Name, p, string(debug.Stack())) + if !act.isroot { + // This line allows to display "hidden" panic with analyzers like buildssa. + // Some linters are dependent of sub-analyzers but when a sub-analyzer fails the linter is not aware of that, + // this results to another panic (ex: "interface conversion: interface {} is nil, not *buildssa.SSA"). + act.r.log.Errorf("%s: panic during analysis: %v, %s", act.a.Name, p, string(debug.Stack())) + } act.err = errorutil.NewPanicError(fmt.Sprintf("%s: package %q (isInitialPkg: %t, needAnalyzeSource: %t): %s", act.a.Name, act.pkg.Name, act.isInitialPkg, act.needAnalyzeSource, p), debug.Stack())