sewenew
commented
2 years ago Since redis-plus-plus has gone down the return-code route of error handling
NO. In fact, redis-plus-plus uses exception, instead of return code, to report error. Some Redis' methods return bool. However, when it returns false, it doesn't mean that some error happens. Instead, it means Redis server returns an integer reply of 0.
Redis protocol (RESP 2) doesn't have bool reply. I converted an integer reply to a bool reply, if and only if the integer reply can either be 1 or 0. However, that seems to be a bad idea. Since Redis might change the command to return other integers (with some new options) in some future version. And there's plan to correct this behavior, i.e. revert these methods to return long long instead.
There's an exception, the SET command. If client sends it without NX or XX, it always returns "OK" status reply. Otherwise, it might return nil reply (if the specified option doesn't match). In this case, I converted "OK" status as true, and nil reply as false.
C++17 added the [[nodiscard]] attribute
I like this feature. However, I'm not sure whether it's still helpful, if all bool-returning methods have been changed to long long-returning method. Also it might not be appropriate for SET command. Since in most cases, i.e. without NX or XX option, we don't need to check the return value. [[nodiscard]] might add some burden to users.
So I think it might be better to postpone the decision until we modify bool-returning methods.
Anyway, thanks for your suggestion and sorry for the late reply!
Regards
Is your feature request related to a problem? Please describe. Since redis-plus-plus has gone down the return-code route of error handling, it is very easy to forget to check a return code, and therefore for failures to be ignored. This has caused countless bugs and security holes in C code, and requires careful reading through of the code to make sure that every case has been handled.
Describe the solution you'd like Luckily, we can very easily flag up cases where developers have missed handling the return code. C++17 added the
[[nodiscard]]attribute, and many platforms have their own versions predating this standard.I'd be very happy to make a pull request for this, but I want to see if a) this is wanted, and b) if there are any further ideas people have.
Describe alternatives you've considered Of course, well-written and coordinated programs would not require this, but I am yet to meet a developer who doesn't occasionally make mistakes like this in a rush. Adding a warning would eliminate practically all such mistakes, and in the rare cases where checking it is not required, a simple
(void)cast will both silence the warning and indicate intention.Additional context For the sake of diplomacy, I'd like to state that I have no issue with return codes over exceptions, and would 100% agree with the choice made with this library.