Describe the bug

redis-cli exit code can be 1 or 255 on failure. Looks like it's inconsistent, not sure when to expect one or another.

To reproduce

This is how 255 is returned:

https://github.com/redis/redis/blob/b69636d377cce0eb8c1fae3ac80240387e31d78f/src/redis-cli.c#L9048

noninteractive() returns REDIS_ERR on failure, which is -1, it becomes 255 as exit codes are in range 0-255.

Expected behavior

As far as I see, nothing is wrong by doing this but it'd nice to be consistent. If we want to have a quick fix and use exit code 1 on all cases, we might be introducing a breaking change? I'm not sure if anybody relies on this.

Also, we had a quick conversation with @yossigo, we may further improve error codes by using different exit codes for different error cases as well.

Comment From: hwware

I agree with you, keeping consistent always is a good style for a software, especially for error code which could give developer a clear what happened.

Comment From: oranagra

@tezc i merged your PR, so now 7.0 would behave the same as 6.2. but IIUC there are still places where we do (and always did) return 255 by mistake? i personally think this is ok to change, and i also think that unless we have some documentation that explicitly defines what each value means, users should just do !=0 rather than ==1.

Comment From: tezc

@oranagra I agree with you %100 user should check != 0 and I feel like it should be okay to change 255 exit codes to 1.

@yossigo thought returning 255 instead of 1 on connection failure would be a breaking change, so I created the PR.

Now, thinking the other way around, if we fix places and return 1 instead of 255, would that be a breaking change as well?

As you said people should check !=0, even they don't do that, they should be okay with 1 as it is the generic error code. 255 is considered out of range error code IIUC, it's definitely not something to rely on, so I believe we can change it to 1.

@yossigo wdyt?

Comment From: yossigo

I think we can live with this breaking change and always return 1. In the future I'd be happy to introduce more formal exit codes but we don't have to do that now.

Comment From: tezc

Created a PR for it: https://github.com/redis/redis/pull/10468