feat: conditionally panic-recover (projectdiscovery#5553)

* feat: conditionally panic-recover As discussed with @Mzack9999, we should avoid overusing panic-recover. We need to review the RCA first to determine whether this is an exceptional situation or if it's a higher-level function meant to recover from a panic. This approach will help us establish a robust error-handling strategy. The implementation of panic-recover should be conditional and NOT applied when running in a CI environment AND IS temporary. Once we've caught all errors and made the necessary corrections, we can remove the deferred recover function. Signed-off-by: Dwi Siswanto <[email protected]> * chore(deps): bump `go-ci` to v1.0.2 Signed-off-by: Dwi Siswanto <[email protected]> * chore(make): add `-race` to `GOFLAGS` in `test` Signed-off-by: Dwi Siswanto <[email protected]> --------- Signed-off-by: Dwi Siswanto <[email protected]>
alban-stourbe-wmx · Aug 28, 2024 · e0b2542 · e0b2542
1 parent 6b71af4
commit e0b2542
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 0 deletions.
diff --git a/Makefile b/Makefile
@@ -65,6 +65,7 @@ docs:
 
 	git reset --hard # line 59
 
+test: GOFLAGS = -race -v
 test:
 	$(GOTEST) $(GOFLAGS) ./...
 

diff --git a/go.mod b/go.mod
@@ -177,6 +177,7 @@ require (
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/kataras/jwt v0.1.10 // indirect
+	github.com/kitabisa/go-ci v1.0.2 // indirect
 	github.com/klauspost/compress v1.17.8 // indirect
 	github.com/klauspost/pgzip v1.2.6 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect

diff --git a/go.sum b/go.sum
@@ -625,6 +625,10 @@ github.com/kevinburke/ssh_config v1.2.0/go.mod h1:CT57kijsi8u/K/BOFA39wgDQJ9CxiF
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
+github.com/kitabisa/go-ci v1.0.1 h1://FHQzlDqYW+3qr0judsOE9X2ZrdRlRc66sCaVrLFGc=
+github.com/kitabisa/go-ci v1.0.1/go.mod h1:4MWu+kf/+tvd0vLWSJA689Kn+hrYkZiymmZYT5BGT4g=
+github.com/kitabisa/go-ci v1.0.2 h1:rqHf8KEbQOxVb998TbqGRo70Z7ol44io7/jLYJUvKp8=
+github.com/kitabisa/go-ci v1.0.2/go.mod h1:e3wBSzaJbcifXrr/Gw2ZBLn44MmeqP5WySwXyHlCK/U=
 github.com/klauspost/compress v1.4.1/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU=

diff --git a/pkg/js/compiler/compiler.go b/pkg/js/compiler/compiler.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	"github.com/dop251/goja"
+	"github.com/kitabisa/go-ci"
 
 	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/generators"
 	"github.com/projectdiscovery/nuclei/v3/pkg/types"
@@ -120,11 +121,17 @@ func (c *Compiler) ExecuteWithOptions(program *goja.Program, args *ExecuteArgs,
 	defer cancel()
 	// execute the script
 	results, err := contextutil.ExecFuncWithTwoReturns(ctx, func() (val goja.Value, err error) {
+		// TODO(dwisiswant0): remove this once we get the RCA.
 		defer func() {
+			if ci.IsCI() {
+				return
+			}
+
 			if r := recover(); r != nil {
 				err = fmt.Errorf("panic: %v", r)
 			}
 		}()
+
 		return ExecuteProgram(program, args, opts)
 	})
 	if err != nil {

diff --git a/pkg/js/compiler/pool.go b/pkg/js/compiler/pool.go
@@ -11,6 +11,7 @@ import (
 	"github.com/dop251/goja"
 	"github.com/dop251/goja_nodejs/console"
 	"github.com/dop251/goja_nodejs/require"
+	"github.com/kitabisa/go-ci"
 	"github.com/projectdiscovery/gologger"
 	_ "github.com/projectdiscovery/nuclei/v3/pkg/js/generated/go/libbytes"
 	_ "github.com/projectdiscovery/nuclei/v3/pkg/js/generated/go/libfs"
@@ -84,11 +85,18 @@ func executeWithRuntime(runtime *goja.Runtime, p *goja.Program, args *ExecuteArg
 			opts.Cleanup(runtime)
 		}
 	}()
+
+	// TODO(dwisiswant0): remove this once we get the RCA.
 	defer func() {
+		if ci.IsCI() {
+			return
+		}
+
 		if r := recover(); r != nil {
 			err = fmt.Errorf("panic: %s", r)
 		}
 	}()
+
 	// set template ctx
 	_ = runtime.Set("template", args.TemplateCtx)
 	// set args

diff --git a/pkg/protocols/headless/engine/page_actions.go b/pkg/protocols/headless/engine/page_actions.go
@@ -14,6 +14,7 @@ import (
 	"github.com/go-rod/rod/lib/input"
 	"github.com/go-rod/rod/lib/proto"
 	"github.com/go-rod/rod/lib/utils"
+	"github.com/kitabisa/go-ci"
 	"github.com/pkg/errors"
 	"github.com/projectdiscovery/gologger"
 	"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/contextargs"
@@ -54,7 +55,12 @@ func (p *Page) ExecuteActions(input *contextargs.Context, actions []*Action, var
 	waitFuncs := make([]func() error, 0)
 
 	// avoid any future panics caused due to go-rod library
+	// TODO(dwisiswant0): remove this once we get the RCA.
 	defer func() {
+		if ci.IsCI() {
+			return
+		}
+
 		if r := recover(); r != nil {
 			err = errorutil.New("panic on headless action: %v", r)
 		}

diff --git a/pkg/tmplexec/flow/flow_executor.go b/pkg/tmplexec/flow/flow_executor.go
@@ -13,6 +13,7 @@ import (
 	"github.com/projectdiscovery/nuclei/v3/pkg/scan"
 	templateTypes "github.com/projectdiscovery/nuclei/v3/pkg/templates/types"
 
+	"github.com/kitabisa/go-ci"
 	"github.com/projectdiscovery/nuclei/v3/pkg/types"
 	errorutil "github.com/projectdiscovery/utils/errors"
 	fileutil "github.com/projectdiscovery/utils/file"
@@ -201,7 +202,13 @@ func (f *FlowExecutor) ExecuteWithResults(ctx *scan.ScanContext) error {
 		}
 
 	}()
+
+	// TODO(dwisiswant0): remove this once we get the RCA.
 	defer func() {
+		if ci.IsCI() {
+			return
+		}
+
 		if r := recover(); r != nil {
 			f.ctx.LogError(fmt.Errorf("panic occurred while executing flow: %v", r))
 		}
-Original file line number
+Diff line change
@@ Expand Up / @@ -65,6 +65,7 @@ docs: @@
     	git reset --hard # line 59
+    test: GOFLAGS = -race -v
     test:
     	$(GOTEST) $(GOFLAGS) ./...
@@ Expand Down @@