Feathers-sync Redis Adapter with an existing Redis Sentinel client and Feathers-Socket.io

zecarlos94 commented 1 year ago

Steps to reproduce

We're using feathers-sync sync using a redis connectionString to synchronize multiple feathers instances and feathers-socket.io to publish events that are consumed by other services as it follows,

import { feathers } from '@feathersjs/feathers';
import sync from 'feathers-sync';
import express from '@feathersjs/express';
import Redis from 'ioredis';
import socketio from '@feathersjs/socketio';

const app: Application = express(feathers());

const redisClient = new Redis(connectionString);

redisClient.on('ready', () => {
  console.log('Redis server is ready.');
});

redisClient.on('error', error => {
  console.log(`Error in Redis server: ${error}`);
});

app.configure(
  sync({
    uri: connectionString
  })
);

app.configure(
  socketio({
    cors: {
      origin: '*'
    }
  });

We need to change this sync mechanism to support Redis Sentinel. We've found the following issues and pull requests that are supposed to fully support this change (use existing redis client as opposed to create a new one using a given connectionString).

However, when we changed this sync mechanism config to use the redis adapter we stop receiving socket updates. We've bumped feathers version 5.0.0-pre.17 to 5.0.0-pre.34, removed lazyConnect option and invoked connect() afterwards, and the issue remains.

import { feathers } from '@feathersjs/feathers';
import { redis } from 'feathers-sync';
import express from '@feathersjs/express';
import Redis from 'ioredis';
import socketio from '@feathersjs/socketio';

const app: Application = express(feathers());

const useSentinelMode: boolean = ....; // obtained from environment variable

const redisClient = useSentinelMode ? 
  new Redis({
      sentinels: [
        { host: 'a...', port: 26379 },
        { host: 'b...', port: 26379 },
        { host: 'c...', port: 26379 },
      ],
      connectionName: 'testConnection',
      db: 4,
      name: 'testApp',
      password: 'password',
      sentinelPassword: 'sentinelPassword',
      commandTimeout: 5000,
      timeout: 5000,
      lazyConnect: true
  }) :
  new Redis({
      connectionName: 'testConnection',
      commandTimeout: 5000,
      timeout: 5000,
      lazyConnect: true
  });

redisClient.on('ready', () => {
  console.log('Redis server is ready.');
});

redisClient.on('error', error => {
  console.log(`Error in Redis server: ${error}`);
});

app.configure(
  redis({
    redisClient,
    key: 'feathers-sync',
    deserialize: JSON.parse, // without this it throws serialize is not a function
    serialize: JSON.stringify // without this  it throws serialize is not a function
  })
);

app.configure(
  socketio({
    cors: {
      origin: '*'
    }
  })
);

app.sync.ready.then(() => console.log('feathers sync ready has been resolved'));

app.on('sync-out', (data) => {
  console.log('sync-out data: ', data);
});

If we remove this sync mechanism configuration, we receive socket updates as we received when using standalone redis connection (with a given connection string).

app.configure(
  redis({
    redisClient,
    key: 'feathers-sync',
    deserialize: JSON.parse, // without this it throws serialize is not a function
    serialize: JSON.stringify // without this  it throws serialize is not a function
  })
);

Module versions (especially the part that's not working): "@feathersjs/authentication": "^5.0.0-pre.34", "@feathersjs/configuration": "^5.0.0-pre.34", "@feathersjs/errors": "^5.0.0-pre.34", "@feathersjs/express": "^5.0.0-pre.34", "@feathersjs/feathers": "^5.0.0-pre.34", "@feathersjs/socketio": "^5.0.0-pre.34", "@feathersjs/transport-commons": "^5.0.0-pre.34", "feathers-knex": "^8.0.1", "feathers-profiler": "^0.1.5", "feathers-sync": "^3.0.1", "ioredis": "^5.2.4", "@types/ioredis": "^4.28.10",

NodeJS version: node -v -> 18.13.0 npm -v -> 8.19.3

Expected behavior

Redis client is instantiated correctly
Redis server is ready on 'ready' events
feathers-sync ready promise is resolved successfully (app.sync.ready below)
Publishing key feathers-sync to Redis -> publish sync-out (log redis adapter do feathers-sync)
sync-out data: {....} // data is an object with correct info
socket (configured feathers socket.io) updates are propagated

Actual behavior

Redis client is instantiated correctly
Redis server is ready on 'ready' events
feathers-sync ready promise is resolved successfully (app.sync.ready below)
Publishing key feathers-sync to Redis -> publish sync-out (log redis adapter do feathers-sync)
sync-out data: {....} // data is an object with correct info
socket (configured feathers socket.io) updates are not propagated nor sync-in events are logged

System configuration

Operating System: Linux

Browser Version: Chrome Version 108.0.5359.124 (Official Build) (arm64)

zecarlos94 commented 1 year ago

Since we're using ioredis we could not use existing redis adapter. It was necessary to implement a custom one.

daffl commented 1 year ago

What were the differences? It probably makes sense to have the ioredis driver working.

feathersjs-ecosystem / feathers-sync