Call Safe functions from panicing ones about casbin HOT 7 CLOSED

I once tried to call error returned functions and panic if error encountered. But I found an issue: the call stack is lost. When an internal panic occurs, it usually carries with the call stack when the panic occurs. But if it is turned into an error, and panic the error. Then call stack will only be the code line that called panic(err). The original faulty call stack is lost. This is inconvenient for debugging errors.

If we make error returning functions the regular ones, some APIs have to be broken: Enforce() has to return bool, error instead of bool. And the caller has to write: result, err := e.Enforce(xx, xx, xx) instead of simple if e.Enforce(xx, xx, xx). I think this will complicate the usage? As far as I know, although the safe functions are already provided, no project in GitHub ever uses them. It seems that most people don't like to use complicated things?

from casbin.

Using panic might be useful for debugging, but I'm pretty sure I would never use panics in a live application. It takes the flow control from you. I know Go's error handling is quite unusal, but using panics is not a good idea either IMO.

from casbin.

OK. Can you make a PR about it?

from casbin.

Many would say that Panic should only be used when the service is unrecoverable and thus should go down.

It's also a way to indicate that something impossible has happened ... 
This is only an example but real library functions should avoid panic...

One possible counterexample is during initialization: if the library truly cannot set itself up, it might be reasonable to panic...

https://golang.org/doc/effective_go.html#panic

See also:
[1]: https://go-proverbs.github.io/
[2]: https://github.com/golang/go/wiki/CodeReviewComments#dont-panic

from casbin.

@sagikazarmark @MaerF0x0 I have a new idea. We can return errors and panic at the same time. We set a flag named triggerPanic. Then write code like:

err := xxx
if triggerPanic {
  panic(err)
}
return err

Then I will make triggerPanic to be false by default. So you can get errors as you want. And I can call enforcer.triggerPanic = true and enjoy the panic when something crashes in the test scripts.

from casbin.

There are libraries which append the call stack to the error, i.e. Github.com/pkg/errors

from casbin.

It has been 2 years and Casbin v2.x has supported returning errors instead of causing panic. So no safe functions required any more. Closed.

https://github.com/casbin/casbin/releases/tag/v2.0.0

from casbin.