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

Incompatibility with socket.io-emitter #78

Closed
aPoCoMiLogin opened this issue Dec 2, 2015 · 7 comments
Closed

Incompatibility with socket.io-emitter #78

aPoCoMiLogin opened this issue Dec 2, 2015 · 7 comments

Comments

@aPoCoMiLogin
Copy link

Well socket.io-redis is broken at the moment. Without some workaround you cant even use it: #73 (comment) and there come another issue, which broke compatibility with emits from socket.io-emitter, because of change psubscribe for subscribe: 800ef74 so other socket.io-emitter libs wont work too. If that two packages are related, why there is no tests with that package ?

@aPoCoMiLogin
Copy link
Author

The only way at the moment is to stick with 0.1.3 version, and in options provide just host and port, to let use socket.io-redis his own redis package (which is same as socket.io-emitter). Its the way i get it working for now..

@ghost
Copy link

ghost commented Dec 9, 2015

Yes, python implementation encountered no compatibility problems. 0.1.4-> 0.2.0 also modified ms format.

The day before yesterday to achieve a socket.io-python-emitter compatible modifying 0.2.0, awaiting merged into the trunk. https://github.com/GameXG/socket.io-python-emitter/commit/2ece80ac4cfbe5c64ffccf93175a68e0b66cdd58

Why not do a more curious over it? 0.2.0 channel name prefix plus mark, while receiving the old version of the channel name, and slowly transition it?

@aPoCoMiLogin
Copy link
Author

It's compatible after that commit: socketio/socket.io-redis-emitter@e656600 tested it today, and it works. Though that they will not fix that, cause last commit was a year ago.. So the issue is resolved at the moment.

@toblerpwn
Copy link

@aPoCoMiLogin - fwiw, this was fixed here: socketio/socket.io-redis-emitter#27 and released as socket.io-emitter 0.3.0. your issue is even more apropos since socket.io-redis 0.2.0 was released to npm with this issue, though.

our company (like the unit tests) actually missed this original break in socket.io-emitter after socket.io-redis bumped to 0.2.0, so we had a production issue (WHOOPS).

unsure the best way to ensure this doesn't happen in the future on "peer"-related modules/repos in general, but I'm sure @rauchg and @nkzawa would be curious if you have a suggestion. (unless they already have a "fix" for this type of failure.)

my thoughts: maybe all of these inter-related packages should run their unit tests on the latest versions of each of their dependencies in package.json after any version in the socket.io family is bumped (at least majorly, if not minorly)...but that sounds really onerous, and probably involves some odd dynamic post-processing of the package.json file, and I don't even know if Travis/whatever would support that anyway?

either way: in this one particular case, instead of more testing, perhaps the construction of the base#namespace#room message pattern should just be centralized in socket.io-adapter (if I understand the vision & module relationships properly?), or some other independent dependency. ultimately it feels like protocol/API specifications like this should not be copy-pasted/hard-coded in three (or more?) modules - it's very risky (obviously, since it blew up here), AND makes synchronizing deployments somewhat more complex...so maybe the fix for this is not about more process or more unit tests, but a small architecture change that centralizes message construction?

@aPoCoMiLogin
Copy link
Author

@toblerpwn huh thanks for the response, but I was faster by seconds :D anyway, there should be info in readme that it works with specific versions, and tests were make for specific version of socket.io-redis (talking about socket.io-emitter), then it will be more clear, cause I had to brake down both packages, check tests and compare package.json to find the issue. So the lists of compatible packages with their versions would be good enough, at least for me.

@rauchg
Copy link
Contributor

rauchg commented Dec 10, 2015

@toblerpwn obviously the key is to always pin modules to versions and pay attention to version bumps. Perhaps we should have done 1.0.0 for -redis since it was breaking, and 1.0.0 for emitter. I'll do that now. I apologize for the disruption.

@rauchg
Copy link
Contributor

rauchg commented Dec 10, 2015

Done 💥🎉

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

3 participants