Legacy mode in v5 is not returning promises from the methods?

[jsumners-nr]

In v4 one could do:

const ok = await client.v4.set('key', 'value')
console.log(ok) // true

But in v5, it seems we have to:

const ok = await promisify(client.set)('key', 'value')
console.log(ok)

This seems inconsistent with #2398, in particular the comment #2398 (comment):

Ok, I found the issue, working on a fix (+ I'll add a test to make sure it won't happen again) now

I don't really know what the difference between non-legacy mode and legacy mode is supposed to be (I can't find any docs on it), but it this change feels like a step backward. Is this intentional? Should we really need to wrap the methods for promises?

[nkaradzhov]

Hi @jsumners-nr ,

In v5, there is a new way to get a legacy client, see here.

I ran the following program and didnt see any issues. Can you doublecheck and if the issue persists, can you please provide exact example?

import { createClient } from 'redis';

async function main() {
  const client = createClient();
  await client.connect()

  // normal v5 client
  const res = await client.set('key', 'value');
  console.log(res); // OK

  // create the legacy client 
  const legacyClient = client.legacy();

  // legacy client
  legacyClient.set('key', 'value', (err, reply) => {
    console.log('callback', err, reply); // callback null 'OK'
  });

  // normal v5 client
  const res1 = await client.set('key', 'value');
  console.log(res1); // OK

  //this closes both clients
  await client.close();
}

main();

[jsumners-nr]

I understand that it is different to initialize the legacy mode. I said that the resulting API no longer returns a promise. You show exactly what I wrote.

You wrote:

// legacy client
  legacyClient.set('key', 'value', (err, reply) => {
    console.log('callback', err, reply); // callback null 'OK'
  });

In v4, the callback was not necessary and if it was omitted then a promise was returned. I also linked to your own issues where the maintainer stated that the legacy methods not returning a promise is a bug and that a test would be added to ensure the mistake does not happen again in the future. But v5 makes the same mistake.

[jsumners-nr]

@nkaradzhov can you confirm that this is a bug in v5?

[nkaradzhov]

@jsumners-nr I dont see any issue.

Lets try to clarify how I think things work:

In [email protected]

import { createClient } from 'redis';

async function main() {
  // callback based client ( legacyMode: true ).
  const client = createClient({ legacyMode: true });
  await client.connect();

  // GOOD
  // commands directly invoked on a legacy client will use the callback syntax
  client.set('key', 'value', () => {
    console.log('callback');
  });

  // GOOD
  // to access the promise api you use the v4 property
  const res = await client.v4.set('key', 'value');
  console.log('res', res); //res OK

  // BAD
  // this is wrong - v4 doesnt support callbacks
  client.v4.set('key', 'value', () => {
    console.log('this callback is never called');
  });

  // BAD
  //this is wrong - legacy client doesnt support promises
  const res1 = await client.get('key');
  console.log('res1', res1); //res1 undefined
  
}

main();

In [email protected]

import { createClient } from 'redis';

async function main() {
  // create normal promise based client
  const client = createClient();
  await client.connect();

  // GOOD
  // commands invoked on a normal client will return a promise
  const res = await client.set('key', 'value');
  console.log(res); // OK

  // BAD, normal client doesnt support callbacks
  client.get('key', () => {
    console.log('this callback is never called'); // callback null 'value'
  });

  // to access callback based legacy client
  const legacyClient = client.legacy();

  // GOOD
  legacyClient.set('key', 'value', (err, reply) => {
    console.log('callback', err, reply); // callback null 'OK'
  });

  // GOOD
  legacyClient.get('key', (err, reply) => {
    console.log('callback', err, reply); // callback null 'value'
  });

  // BAD
  // commands invoked on a legacy client will not return a promise
  const promised = await legacyClient.get('key');
  console.log('promised', promised); // undefined

  //this closes both clients
  await client.close();
}

main();

To summarize:

Version	Promise	Callback
v4	client.v4	const client = createClient({ legacyMode: true });
v5	const client = createClient();	const legacy = client.legacy();

[jsumners-nr]

Thank you. After reviewing those examples, I believe our test suite was incorrectly using the v4 legacy API.