Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panics in vacuum goroutines can't be handled #216

Closed
TristanSpeakEasy opened this issue Dec 21, 2022 · 6 comments
Closed

panics in vacuum goroutines can't be handled #216

TristanSpeakEasy opened this issue Dec 21, 2022 · 6 comments

Comments

@TristanSpeakEasy
Copy link
Contributor

If a panic occurs in a custom rule, because it seems the document is being evaluated with goroutines these can't be caught using recover and so can cause crashes for a server.

It would be good if vacuum can recover from panicing goroutines and then throw this panic on the main thread so then these panics can be caught by the code calling vacuum.

here is an example callstack

panic: ...truncated `

goroutine 298 [running]:
... truncated
github.com/daveshanley/vacuum/motor.buildResults({0xc000b70bb0, 0xc0002ce140, {0x12835b0, 0xc000b20d30}, 0xc000b1c450, 0xc000b18500, 0xc000b1c0a8, 0xc000601000, 0xc000b29360, 0xc000b1b470}, ...)
        /home/trist/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.45/motor/rule_applicator.go:387 +0x608
github.com/daveshanley/vacuum/motor.runRule({0xc000b70bb0, 0xc0002ce140, {0x12835b0, 0xc000b20d30}, 0xc000b1c450, 0xc000b18500, 0xc000b1c0a8, 0xc000601000, 0xc000b29360, 0xc000b1b470})
        /home/trist/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.45/motor/rule_applicator.go:314 +0x350
created by github.com/daveshanley/vacuum/motor.ApplyRulesToRuleSet
        /home/trist/go/pkg/mod/github.com/daveshanley/vacuum@v0.0.45/motor/rule_applicator.go:157 +0x72e
exit status 2
@daveshanley
Copy link
Owner

Do you have an example of a custom rule and a sample snippet of a spec that you're using to throw this panic? I want to ensure that I catch this in the right place.

@TristanSpeakEasy
Copy link
Contributor Author

Here is a small reproducible

package main

import (
	"fmt"
	"io"
	"net/http"

	"github.com/daveshanley/vacuum/model"
	"github.com/daveshanley/vacuum/motor"
	"github.com/daveshanley/vacuum/rulesets"
	"gopkg.in/yaml.v3"
)

func read() []byte {
	res, err := http.Get("https://raw.githubusercontent.com/speakeasy-api/openapi-directory/main/APIs/nexmo.com/conversation/2.0.1/openapi.yaml")
	if err != nil {
		panic(err)
	}
	defer res.Body.Close()

	data, err := io.ReadAll(res.Body)
	if err != nil {
		panic(err)
	}

	return data
}

func main() {
	defer func() {
		if r := recover(); r != nil {
			fmt.Println("recovered") // won't capture the panic in the rule function
		}
	}()

	rules := map[string]*model.Rule{
		"test": {
			Id:           "test",
			Formats:      model.OAS3AllFormat,
			Given:        "$",
			Recommended:  true,
			RuleCategory: model.RuleCategories[model.CategoryValidation],
			Type:         "validation",
			Severity:     model.SeverityError,
			Then: model.RuleAction{
				Function: "test",
			},
		},
	}

	set := &rulesets.RuleSet{
		DocumentationURI: "",
		Rules:            rules,
		Description:      "",
	}

	motor.ApplyRulesToRuleSet(
		&motor.RuleSetExecution{
			RuleSet: set,
			Spec:    read(),
			CustomFunctions: map[string]model.RuleFunction{
				"test": &testRule{},
			},
		})
}

type testRule struct{}

func (t *testRule) GetSchema() model.RuleFunctionSchema {
	return model.RuleFunctionSchema{
		Name: "test",
	}
}

func (d *testRule) RunRule(nodes []*yaml.Node,
	context model.RuleFunctionContext,
) []model.RuleFunctionResult {
	panic("run away!")
}

@daveshanley
Copy link
Owner

Great! thank you.

daveshanley added a commit that referenced this issue Dec 21, 2022
When building custom rules programatically, now you can supply a `PanicFunction` handler to `RuleSetExecution`. This allows any panic thrown in a custom function, to be caught upstream and handled appropriately.
@daveshanley
Copy link
Owner

daveshanley commented Dec 21, 2022

I have created a new PanicFunction property that takes a type of func(p: interface{}) on the ResultExecution type.

This function captures any panic thrown by any rule executing and you can do what you like with the panic thrown (p)

 saveMePlease := func(r any) {
      fmt.Printf("panic: saved by the bell (%v)", r) 
  }

  motor.ApplyRulesToRuleSet(
      &motor.RuleSetExecution{
          PanicFunction: saveMePlease,
          RuleSet:       set,
          Spec:          read(),
          CustomFunctions: map[string]model.RuleFunction{
              "test": &testRule{},
          },
      })

Using your custom rule:

type testRule struct{}

func (t *testRule) GetSchema() model.RuleFunctionSchema {
    return model.RuleFunctionSchema{
        Name: "test",
    }
}

func (d *testRule) RunRule(nodes []*yaml.Node,
    context model.RuleFunctionContext,
) []model.RuleFunctionResult {
    panic("run away!")
}

Will output:

panic: saved by the bell (run away!)

#218

daveshanley added a commit that referenced this issue Dec 21, 2022
When building custom rules programatically, now you can supply a `PanicFunction` handler to `RuleSetExecution`. This allows any panic thrown in a custom function, to be caught upstream and handled appropriately.
@daveshanley
Copy link
Owner

This will be available in v0.0.47 once the pipeline finishes running.

@TristanSpeakEasy
Copy link
Contributor Author

works great thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants