EXPIRE deviates from REDIS documented return value (integer reply)

[rafak]

Redis docs for the EXPIRE command state that it returns an "Integer reply"

in node-redis the response is transformed into a boolean.

// NOTE: r - instance of redis client

> r.set('aaa','888').then(console.log)
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 2924,
  [Symbol(trigger_async_id_symbol)]: 2920
}
> OK
> r.get('aaa').then(console.log)
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 2545,
  [Symbol(trigger_async_id_symbol)]: 2541
}
> 888
> r.expire('aaa', -11).then(console.log)
Promise {
  <pending>,
  [Symbol(async_id_symbol)]: 2718,
  [Symbol(trigger_async_id_symbol)]: 2714
}
> true      // <======= returns bool

Environment:

• Node.js Version: v16.18.1
• Redis Server Version: redis_version:6.2.6
• Node Redis Version: redis@4.5.1
• Platform: debian 10 buster (docker)

[leibale]

The reply is either 0 or 1, the server replies with an integer and not a boolean because RESP2 does support booleans..I think converting the reply to a boolean is more convenient.. @chayim @guyroyse @itamarhaber WDUT?

[rafak]

I think converting the reply to a boolean is more convenient

just a note on that from the perspective of a person using the library:

it can be whatever but i would suggest explicitly adding this to the documentation - that the response from node-redis differs from the response from redis itself and also add it to the migration guide as the previous behavior was different

[guyroyse]

I agree with @leibale. If it returned a number I would just convert it to a boolean in my code. (‼️) This is a convenience in the same way that hGetAll returning JavaScript objects instead of an array of strings as the Redis documentation states is a convenience.

That said, is there any chance that Redis might decide to return something more than just 0 or 1? Is there some other state that might be useful here? I'm thinking like how TTL used to return -1 if a key was persistent or didn't exists but then changed to be -2 if it didn't exists and -1 if it existed but was persistent. If so, a boolean would lock us in to a particular pattern that would be a pain to escape.

[itamarhaber]

I don't see EXPIRE's return value being overloaded with -1 in the future, but anything could happen. Also, note that in RESP3 we still use an integer rather than a Boolean. As much as I like this convenience (casting 0/1 to bool) it feels brittle in terms of not being totally future-proof.

[chayim]

I'm very much on board with the client returning a boolean to replace 0/1 when things should be a boolean ideally (i.e EXPIRE), and the fact that RESP2 (and Redis 1.0.0 in this case) didn't have another answer.

We need to consider this API (on the redis side) broken anyways. Let's assume that the reason to maintain this integer use case, is a potential future multi key response. Well, we run into the following:

1. In the multi-key future, Redis could respond with the number of keys deleted, instead of a simple 0/1. If it continues to return 0/1 then bool holds.
2. Let's assume that 0/1 is no longer valid - and since we're multi key, and want to maintain the same return types, redis continues to return an integer as documented. I believe the logical choice would now be for the number of keys deleted. Unfortunately as a user I have to both validate that number against my keys sent. In the case where the number is inaccurate we have a different problem - I now have to step over all of the keys (assumedly in a pipeline) to set EXPIRE again, assuming the keys exist AND I have to process the response.
3. If redis does not want to maintain backwards compatibility, then the right thing to do would be to either return an array of 0/1 integers, an array of booleans for RESP3, or possibly a map of {key: 0/1, key2: 0/1...etc}. One could also return an array of strings for key names, but I'd say this is very heavy weight, wire wise.

The result means that I, as a user have more work to do either way. Hence, voting boolean. I believe commands of this nature are already broken for multikey and may argue that this is equally as future problematic, but at least more user friendy.

[sjpotter]

I think boolean is fine, but not quite for the reason @chayim states. I don't think EXPIRE can ever be multikey, I'd think we would have an MEXPIRE that is multi key, which would then have a different API.

if it were me, I'd update the redis command documentation to say that it returns an integer reply with the boolean values of 0 and 1. emphasis to add the boolean word, to make it clear that it will only ever be 0 or 1). Any other use case should be handled via an error response.