google
uuid.MustParse should panic error in error type
Currently MustParse panic the error in string type,
panic should be an error type, so the recover can have consistance use of err.Error()
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
Normally MustParse is only used when initializing global variables (or init functions). Any place else should probably be done with Parse and then check the error. I don't disagree that panicking with an error is perhaps slightly better. My only question is if it is worth enough to change the API.
@pborman I know that golang does not encourage using defer-catch to throw errors, but after suffering from all of those if err != nil, We decided to use defer-catch patterns to handle errors, especially we are using grpc. we do expect the value from recover being an error, consider the follow code.
func Catch(err *error) {
if r := recover(); r != nil {
panicError := r.(error)
*err = panicError
}
}
if the library panicking with a string, this code will get crash again when type assertion, it will be better if the library can follow the common practice which is panicking an error.
of course we can always check when type assertion
func Catch(err *error) {
if r := recover(); r != nil {
panicError, ok := r.(error)
if !ok {
panicError = fmt.Errorf("%v", r)
}
*err = panicError
}
}
but it feels slightly redundant if user need to do it manually
Hello. I agree with @pborman that changing the value passed to panic now would be backwards incompatible to existing uses of recover catching this panic. There is no requirement that the panic be given an error, a string is allowed. So for these reasons, I will close this. Thanks for the consideration.
