-
Notifications
You must be signed in to change notification settings - Fork 233
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
Require a method to update inner ioSocket object for connect / disconnect #86
Comments
Here's a quick hack that fixes this, by exposing a setter on // socket.js
var wrappedSocket = {
on: addListener,
addListener: addListener,
once: addOnceListener,
socket: function (s) {
if (s) { socket = s; }
return socket;
},
// etc. And the factory above re-worked as such: var socket, token = /* get token */
var wrapper = socketFactory({ ioSocket: socket });
function connect (token) {
socket = io.connect('', { query: 'token=' + token, forceNew: true });
socket.on('connect', function () {
wrapper.socket(socket); // replace with new socket instance
}
}
// other code
return wrapper; So, on a new |
Beauty, this is exactly what I am looking for. I'll let you know if I come up with any efficiency/cleanliness changes. |
this doesn't seem to work, apparently because the forwarding events aren't getting rebound to the socket? In socket.js, the "forward: function(events, scope)" binds events to the socket so a controller can receive them. If you are just replacing the inner socket reference, those bindings get thrown away... |
@ronaldjeremy you are right, it isn't robust. i have run into cases where the the old socket is discarded, and controllers try to forward and they attach to the old discarded socket, then a new socket comes in and you lost the events. i have forked this repo and resolved these issues myself. it didn't appear that brian was super receptive to PRs, and i had to get things done and move forward, so i have a branch here that i am using and it is in production for several months now with no issues. hope it helps. |
I'm receptive to PRs, but I don't have enough time to maintain this library these days. @davisford (and others) – if you're interested I'd be happy to give you push access to this repo if you're interested in helping me maintain it. |
Thanks @btford -- I didn't really mean that as a dig -- poor choice of words. I did send a PR earlier that you dinged for not having a test, and I'm ok with that -- I appreciate you putting this repo out there with an open license, so I don't want to make it seem like I'm complaining. Right now, I'm trying to bootstrap a startup, so my time is pretty limited too; if things change, I'd be happy to try to help maintain. |
Hi @davisford! Your branch works form me, except it connects twice to the server! In the debug is says "Forward has 2 deffered calls b/c socket was not ready yet" I tripple checked thar the connection function is actually only called once. Pls is this the default behavior? What help can you offer? Because now socket.on('disconnect' on the server is getting called twice. |
@coommark is your code on github or somewhere i can take a look at it? also, what branch are you using of the fork? calls made by the controller will be deferred until the socket is ready. that was the goal of the change so that message is not unusual nor does it warrant a problem if you are disconnecting and connecting on a regular basis. the problem initially was that on a disconnect -> reconnect the socket was not ready and clients were trying to use it -- so calls to |
Hi, here is the factory responsible for the connection: ''' var wrapper = socketFactory({ioSocket : socket}) $rootScope.$on('authenticated', function() { return wrapper; ''' |
Kindly disregard the second alert. Creeped into the code after two sleepless nights. |
@coommark i think we should take this off this thread. can you post a gist of your code gist.github.com on a superficial look it looks like you are doing too much in the controller, which will probably get re-initialized on any view change (depends on how you setup angular app, but that is standard way to do things). i think you've got a few things backwards, but it is hard to say without more codebase. post a gist with a few files for me to look at or point me to an open repo, and i'll try to assist. |
Thank you. Been waiting all day. I will do just that. You're very kind. |
So I stripped down the project so that I can git it. When I ran the stripped down version, voila! Connection established only once. Thank you @davisford! I am sure it is a conrib angular module causing the factory to run twice. @btford is there a chance you'll merge @davisford's pull? Because it's awesome to be able to support authentication right out of the box. Thank you guys! |
I did not send a pull request for him to merge, but @mritzco did. Glad you got it worked out. |
@davisford @btford , has @mritzco PR been merged to the project ? And has @mritzco PR solved the forward rebound issue ? thx for answer guys. |
@sam2x Not merged as far as I know, I'm checking the code and the forward event is untouched by my code (socket-deferred line 86), the reason was that the forward function actually belongs to the wrapper not the socket.(line 79) There's a test with forward that's working fine (socket-deferred.spec.js line 226) |
In my app, I have both traditional http and websockets -- both are protected by JSON web token authentication. When a user logs in (i.e. post to http endpoint), a token is returned, and then the client will attempt to connect to the websocket endpoint with the token. The connection is rejected if the token isn't valid.
When a user logs out, the websocket is disconnected. I don't want to leave that channel open anymore b/c the user is no longer authenticated. Likewise, I don't want to re-use that same websocket if a new user logs in, b/c the authentication token will be different (and not valid for the new user).
What I'm left with is something like this:
..and this is an example of how it is in injected into a controller to be used...
On login, the websocket is connected. On logout it is disconnected, and at the end, it is returning the
socketFactory
wrapper.The problem here is that
socketFactory.$get
is only ever called once in the provider, which means that the very first timeSocket
is injected into a controller it is valid and connected. But if a user logs out and logs in again, angular re-injects the same instance into a controller, and the internalioSocket
has since been disconnected and is no longer good. (note the use of socket.ioforceNew
option which abandons the socket wholesale and initiates a brand new connection).What is needed is something like the angular service recipe pattern which news up the object each time, so we don't re-use the same instance of the wrapper...either that or else an API exposed on socketFactory itself that allows one to replace the inner
ioSocket
reference.Any ideas on the best way to accomplish this?
The text was updated successfully, but these errors were encountered: