redis-cli: make returning error exit code possible when command execution fails #8136
redis-cli: make returning error exit code possible when command execution fails #8136oranagra merged 3 commits intoredis:unstablefrom
Conversation
oranagra
added
release-notes
|
thanks @hwware. the but i think it might also be useful to have the return code represent a presence of an error even if we don't abort the execution (i.e. in case the user executed a pipeline of commands and one failed). @redis/core-team please approve the new flag, and share your opinion about the above. |
|
Yep, also very related to #3475 |
yossigo
left a comment
yossigo
left a comment
There was a problem hiding this comment.
@oranagra I agree it can be useful but I don't think we should change the default behavior now as it may have a big negative impact on compatibility.
While at it - we should probably also consider the different high level error cases and map them to error codes.
| fprintf(stderr,"%s\n",reply->str); | ||
| exit(1); |
There was a problem hiding this comment.
@hwware In addition to aborting with an error code, this also introduces other changes as the error is now sent to stderr rather than stdout and it does not conform to the cliFormatReply formatting. Is that intentional? We should consider maintaining compatibility here.
There was a problem hiding this comment.
hi @yossigo , thanks for your comments, yes this is intentional, since when cli abort with error code, the stderr will always been set based on other parts of code, for example when context error happens and we have to exit with an error: https://github.com/redis/redis/blob/unstable/src/redis-cli.c#L873 . the cliFormatReply based on my understanding is formatting reply for "no error", even though this function contains a error case we still don't consider the execution fails and the exit code is still 0.
|
@yossigo by different high level errors, you mean an error to establish connection or authentication error (which means the command wasn't at all executed), should be distinguished from an error response from the command? i understand why my idea of changing the default behavior is wrong, i just think it's a shame that for a single command the default is to return 0 even though it failed. |
|
Hi @oranagra , I agree with @yossigo 's points regarding the default behaviour when command execution fails. Since this may cause big compatability issue if user already starting using current version. But we can centainy provide another flag controlling the return and it won't break the backward compatability (maybe we have too many flags IMHO and you don't think we should do this) Or we can think about this later, maybe we need to refactor the cli code for error handling part.. |
…fails (redis#8136) without this option, redis-cli returns 0 even if command fails. kept this for backwards compatibility.
5 participants
This PR fixes #8127, by providing -e option, user can control whether return error code if the command execution fails. @oranagra please take a look and let me know if i miss anything, thanks
Example:
./redis-cli config set requirepass 1234
OK
./redis-cli -e flushdb
NOAUTH Authentication required.
echo $?
1