Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync drops messages when using ioredis client #190

Open
jamesholcomb opened this issue Apr 7, 2023 · 3 comments
Open

sync drops messages when using ioredis client #190

jamesholcomb opened this issue Apr 7, 2023 · 3 comments

Comments

@jamesholcomb
Copy link

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

  • add "ioredis": "^5.3.1" to dev deps
  • add below /test/ioredis.test.js
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
Copy link
Author

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

@daffl
Copy link
Member

daffl commented Apr 7, 2023

Can you put this into a pull request?

@jamesholcomb
Copy link
Author

i will give it a shot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants