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

Avoid panics #14

Open
cheatsnake opened this issue Oct 21, 2023 · 4 comments
Open

Avoid panics #14

cheatsnake opened this issue Oct 21, 2023 · 4 comments

Comments

@cheatsnake
Copy link

A fairly trivial check of division by zero causes panic. Do you plan any protection against such cases? And a way to avoid panics?

panic: runtime error: integer divide by zero [recovered]
        panic: runtime error: integer divide by zero

goroutine 170 [running]:
github.com/maja42/goval/internal.Evaluate.func1()
        /home/user/go/pkg/mod/github.com/maja42/goval@v1.3.1/internal/evaluate.go:11 +0xa5
panic({0x69d820?, 0x9198b0?})
        /usr/local/go/src/runtime/panic.go:914 +0x21f
github.com/maja42/goval/internal.div({0x68b580?, 0x8f77e0?}, {0x68b580?, 0x8f77c0?})
        /home/user/go/pkg/mod/github.com/maja42/goval@v1.3.1/internal/parserUtils.go:207 +0x1cd
github.com/maja42/goval/internal.(*yyParserImpl).Parse(0xc000235200, {0x76a720?, 0xc000230240?})
        parser.go.y:100 +0x132e
github.com/maja42/goval/internal.Evaluate({0xc00020a378?, 0x406f17?}, 0xc0002448a0?, 0x7674b0?)
        /home/user/go/pkg/mod/github.com/maja42/goval@v1.3.1/internal/evaluate.go:18 +0x85
github.com/maja42/goval.(*Evaluator).Evaluate(...)
        /home/user/go/pkg/mod/github.com/maja42/goval@v1.3.1/evaluator.go:31
@maja42
Copy link
Owner

maja42 commented Oct 21, 2023

Adding a check and returning a specific error instead of a panic would indeed be possible.
But it doesn't protect against all panics (called functions can also panic).
Would there be a benefit of handling this case specifically?

@maja42 maja42 closed this as completed Oct 21, 2023
@maja42 maja42 reopened this Oct 21, 2023
@cheatsnake
Copy link
Author

cheatsnake commented Oct 21, 2023

Adding a check and returning a specific error instead of a panic would indeed be possible. But it doesn't protect against all panics (called functions can also panic). Would there be a benefit of handling this case specifically?

Perhaps some special mode (e.g. you can call it safe or limited) where only those actions will be performed that are guaranteed not to cause panic, but only throw an error, would be a good solution.

eval := goval.NewSafeEvaluator()
result, err := eval.Evaluate("2 / 0")

@sashakoshka
Copy link

called functions can also panic

It is possible to ensure called functions do not panic, because the functions do not originate from this module and are added at the application developer's discretion.

Would there be a benefit of handling this case

The evaluated expression may be something entered by a user, and there is no good way to check if the user input contains divisions by zero without re-implementing the functionality in this module. The expected behavior of a program in this scenario is to display a readable error to the user, not a panic, which would be far less awkward if Evaluate returned a division by zero error.

@maja42
Copy link
Owner

maja42 commented Jun 6, 2024

Detecting division by zero sounds reasonable. I'll take a look at it once I have some spare time.

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

3 participants