feathersjs-ecosystem / feathers-sync

Synchronize service events between Feathers application instances
MIT License
222 stars 41 forks source link

sync drops messages when using ioredis client #190

Open jamesholcomb opened 1 year ago

jamesholcomb commented 1 year ago

Trying to utilize ioredis for the redisClient option (I depend on sentinels which is unsupported by node-redis

functions as expected with feathers@4 and feathers-sync@2:

  .configure(
    sync.redis({
      key: app.get("feathers").sync.key,
      redisClient: new Redis({
        // retry forever
        lazyConnect: true,
        maxRetriesPerRequest: null
      }),
      serialize: JSON.stringify,
      deserialize: JSON.parse
    })
  )

but perhaps I missing something for feathers-sync@3?

Steps to reproduce

const assert = require('assert');
const feathers = require('@feathersjs/feathers');
const { redis } = require('../../lib');
const Redis = require('ioredis');

const redisClient = new Redis({
  lazyConnect: true,
  maxRetriesPerRequest: null
});

describe('ioredis tests', () => {
  let app;

  before(done => {
    app = feathers()
      .configure(redis({
        serialize: JSON.stringify,
        deserialize: JSON.parse,
        redisClient
      }))
      .use('/todo', {
        events: ['custom'],
        async create (data) {
          return data;
        }
      });
    app.sync.ready.then(() => {
      done();
    });
  });

  it('duplicates client', () => {
    assert.ok(redisClient.duplicate());
  });

  it('syncs on created with hook context', done => {
    const original = { test: 'data' };

    app.service('todo').once('created', (data, context) => {
      assert.deepStrictEqual(original, data);
      assert.ok(context);
      assert.deepStrictEqual(context.result, data);
      assert.strictEqual(context.method, 'create');
      assert.strictEqual(context.type, 'around');
      assert.strictEqual(context.service, app.service('todo'));
      assert.strictEqual(context.app, app);

      done();
    });

    app.service('todo')
      .create(original)
      .then(data =>
        assert.deepStrictEqual(original, data)
      )
      .catch(done);
  });

  it('sends sync-out for service events', done => {
    const message = { message: 'This is a test' };

    app.once('sync-out', data => {
      try {
        assert.deepStrictEqual(data, {
          event: 'created',
          path: 'todo',
          data: message,
          context: {
            arguments: [message, {}],
            data: message,
            params: {},
            type: 'around',
            statusCode: undefined,
            method: 'create',
            event: 'created',
            path: 'todo',
            result: message
          }
        });
        done();
      } catch (error) {
        done(error);
      }
    });

    app.service('todo').create(message);
  });
});

Expected behavior

Test complete

Actual behavior

Test fail

     AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ '{"event":"created","path":"todo","data":{"message":"This is a test"},"context":{"arguments":[{"message":"This is a test"},{}],"event":"created","result":{"message":"This is a test"},"data":{"message":"This is a test"},"params":{},"path":"todo","method":"create","type":"around"}}'
- {
-   context: {
-     arguments: [
-       {
-         message: 'This is a test'
-       },
-       {}
-     ],
-     data: {
-       message: 'This is a test'
-     },
-     event: 'created',
-     method: 'create',
-     params: {},
-     path: 'todo',
-     result: {
-       message: 'This is a test'
-     },
-     statusCode: undefined,
-     type: 'around'
-   },
-   data: {
-     message: 'This is a test'
-   },
-   event: 'created',
-   path: 'todo'
- }
      at Feathers.<anonymous> (test/adapters/ioredis.test.js:36:16)
      at Object.onceWrapper (node:events:628:26)
      at Feathers.emit (node:events:525:35)
      at service.emit (lib/core.js:2:3446)
      at /Users/jamesholcomb/code/feathers-sync/node_modules/@feathersjs/feathers/lib/events.js:15:55
      at Array.forEach (<anonymous>)
      at /Users/jamesholcomb/code/feathers-sync/node_modules/@feathersjs/feathers/lib/events.js:15:21

System configuration

"@feathersjs/authentication": "5.0.3",
"@feathersjs/authentication-oauth": "5.0.3",
"@feathersjs/configuration": "5.0.3",
"@feathersjs/errors": "5.0.3",
"@feathersjs/express": "5.0.3",
"@feathersjs/feathers": "5.0.3",
"@feathersjs/schema": "5.0.3",
"@feathersjs/socketio": "5.0.3",
"@feathersjs/typebox": "5.0.3",
"feathers-hooks-common": "7.0.3",
"feathers-sync": "3.0.3",

NodeJS version:

18.12.1

jamesholcomb commented 1 year ago

Also, the test syncs on created with hook context times out.

daffl commented 1 year ago

Can you put this into a pull request?

jamesholcomb commented 1 year ago

i will give it a shot