davinkevin / AngularStompDK

Angular service to communicate to Stomp-Websocket
http://davinkevin.github.io/AngularStompDK/
Apache License 2.0
36 stars 12 forks source link

When using delayed connect, each connect() including for subscriptions to channels creates a new connection #45

Closed ghost closed 7 years ago

ghost commented 7 years ago

Hello, Thanks for adding delayed connect! However now I have literally hundreds of copies of each message being delivered if I subscribe to a whole bunch of channels. Looking at RabbitMQ control panel, you see what is in the image. That is with just one user connected. connection number error And it will just keep going up as the page is reloaded! lots of duplicate connectiosn

davinkevin commented 7 years ago

Could you show me the code you use to connect with delay ?

I didn't have this kind of problem, so I would like to be able to reproduce this before trying to fix 😄

ghost commented 7 years ago

With AutoLogin on (!), having already logged in, and being done in a controller,

    x.stompSubscribeChannels = function(channels) {
        for (var i = 0; i < channels.length; i++) {
            if (channels[i] in $rootScope.chsubs)
                continue;
            $rootScope.chsubs[channels[i]] = true;
            ngstomp
            .subscribeTo(channels[i])
                .callback(messageCallback)
            .connect();
        }
    }

This gets called with 3 channels, then once more each time the page is reloaded or we move to another controller, for a total of 3-5 each time. Over time it really adds up, adding 3-5 more connections each time. When I controlled my deferred connect()'s worse, it would be hundreds quite quickly, because I would connect() all over the place. Now the only places connect() happens is during channel subscription, or ONCE if login is deferred. But in this case autologin is used:

    ngstompProvider
        .url(myurl)
        .credential(uname,pass)
        .vhost('xyz')
               .heartbeat('10000','10000')
        .autoConnect(true)
        .class(SockJS);
davinkevin commented 7 years ago

You don't use the syntax with the bindTo with a scope as parameter, so the subscription is never destroyed between controller... Alternatively, the connect method return an object with a method unSubscribeAll() to unregister all the subscription previously done.

ngstomp
        .subscribeTo('/topic/item')
            .callback(whatToDoWhenMessageComming)
            .withBodyInJson()
            .bindTo($scope)
        .connect();
ghost commented 7 years ago

The behavior still continues. This is the very strange thing to me: messages are still only sent once, no matter what. They are just received in duplicate a WHOLE BUNCH.

davinkevin commented 7 years ago

And if you try to do something like this ? (I don't test it, just wrote it in here, so maybe some syntax error).

x.stompSubscribeChannels = function(channels) {
       var aConnection;
        for (var i = 0; i < channels.length; i++) {
            if (aConnection) {
                aConnection = aConnection
                      .and()
                      .subscribeTo(channels[i])
                     .callback(messageCallback)
           } else {
              aConnection = ngStomp
                       .subscribeTo(channels[i])
                       .callback(messageCallback)
           }
        }

       aConnection.connect();

    }

Because here, you have only one connect, so the object return after the connect can disconnect all the subscription previously created.

In your code, you have access to only the last connection

ghost commented 7 years ago

Are you saying that it is expected behavior for more than one connect() to make more than connection to the same host/etc.? If so, I will work around that in my code. But in the previous version, you could do as many subscribe()/.connects() as you wanted to and never run into any trouble; I figured it would be the same in this version.

davinkevin commented 7 years ago

I didn't understand your first sentence...

But, due to modification in last commit, now the connect() method of ngStomp totally change. You can see it here : https://github.com/davinkevin/AngularStompDK/commit/44692c0cf4e2c1aef483f3f24055421193f3b617#diff-fda0006619115788533ce239e0f90be9

But, in your case, this is linked to the connect() method of the builder, and this part doesn't change at all in the last version.

The only modification made is about the system of autoConnect. Do you use the connect method of the ngStomp service at multiple place in your app ? This could cause this kind of problem...

ghost commented 7 years ago

Sorry, it turns out that the multiple-reception issue was on my backend. Want me to come back to this same thread when I troubleshoot the code path that doesn't auto-connect, or add a new one?

davinkevin commented 7 years ago

A new one would be good. I let you close this one 😉