search

feathersjs-ecosystem / feathers-sync

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

Hook is not being emitted #32

Closed PedroMD closed 6 years ago

PedroMD commented 7 years ago

Hello everybody,

Been testing this and everything was fine until i noticed my hook was always undefined when adding filters to my events. I've forked it and hook is now being emitted as well, but I'm not sure this is the right approach: https://github.com/PedroMD/feathers-sync/commit/ce23831f8ec1fa594208f712808a44148401a3c7

I haven't submitted a PR yet, because it is causing issues with socket.io, whenever a client connects to it:

RangeError: Maximum call stack size exceeded at _hasBinary (/home/ubuntu/apps/x-api/node_modules/socket.io/node_modules/has-binary/index.js:25:22) at _hasBinary (/home/ubuntu/apps/x-api/node_modules/socket.io/node_modules/has-binary/index.js:49:63) at _hasBinary (/home/ubuntu/apps/x-api/node_modules/socket.io/node_modules/has-binary/index.js:49:63)

EDIT: Looks like hook can't be passed alongside the data, as the hook obj has some circular references, resulting in recursive calls that exceeded the stack size: https://github.com/socketio/socket.io/issues/1665

Any thoughts on how to solve this? For now, I'm thinking of switching to primus and just use engine.io as the framework (I need polling-failover on my project, as having websocket as the only transport protocol is not an option). Not sure if engine.io will do the trick, but we'll see...

ekryski commented 7 years ago

@PedroMD the hook is not intended to be emitted. What's the use you've got where you need the hook object synchronized across machines? I'm wondering if we are missing something or if you are trying to do something that feathers-sync isn't intended to do.

daffl commented 7 years ago

The problem is that usually on the server you get something like

app.service('myservice').on('created', function(data, hook) {});

But feathers-sync does not send the hook object.

Side note: Currently the hook object is not the same one from the actual hook chain but that will be solved with https://github.com/feathersjs/feathers/issues/408.

PedroMD commented 7 years ago

@daffl, got it :) I've noticed this issue exactly when I was setting up some custom event filters. hook is always undefined if feathers-sync is enabled...

Currently, I don't need the hook object on these filters, so I haven't tested my quick fix with another websocket framework. If I do, I will update this.

daffl commented 6 years ago

This will be fixed in the next release. Caveat is that your hook context (without app and service) must be JSON serializable (so no circular dependencies).