3 ms·
Generally I agree with you and tend to avoid writing or calling functions that can panic. I make a slight exception in this case because it will "never" hit th
by jrockway 6y ago
Generally I agree with you and tend to avoid writing or calling functions that can panic. I make a slight exception in this case because it will "never" hit the error case; you are "always" calling the RPC through an interceptor that adds the necessary value to the context, so your MustGet functions will "always" work. I use "quotes" around never and always because ... sometimes someone edits the code to delete this invariant, and it is very easy to do that. But, it explodes loudly when it panics, so at least you can go in and fix the problem without much effort. (It would be really insidious to return an error and not check it -- some code three functions deep would then blow up with a nil pointer exception; or to return a subtly-not-working default value that cause subtle broken behavior that's hard to detect. Panics are preferred because the program crashes at the exact line of code that's broken.)
For any case where the error would depend on runtime input, rather than compile-time structure, you should return and check an error. If hitting the error case means "we need to edit the code and release a new version", panic is a fine tool to surface that problem.
All in all, I'd basically be happy either way. If you make your GetFoo function return an error that all the handlers check, I'd approve that PR. If you judiciously panic when something that can "never" happen happens, I'd approve that PR. The obvious preference is to update the codegen to pass those parameters in explicitly, so that if the interceptor structure changes, the program will simply not compile. But, the design of things like net/http and grpc kind of preclude easily doing that, and I'm not sure it's worth the effort to write something on top of those that fix the problem, when it's simple enough to do the unpacking manually and your integration tests test it for free anyway.
- sneak 6y agoThat’s a wonderful explanation, thank you.