in JS client when transport fallback OnConnected event in server could get called 2 times for same connection id #2500

Closed
halter73 opened this Issue Sep 5, 2013 · 1 comment

Projects

None yet

4 participants

@halter73
Member
halter73 commented Sep 5, 2013

Functional impact:
in JS client when transport fallback OnConnected event in server could get called 2 times for same connection id

Repro:
1). in Tests.Common, EchoHub, add OnConnected event:

public override Task OnConnected()
        {
            return Clients.All.echo(Context.ConnectionId + " OnConnected");
        }

2). then add below for JS test:

QUnit.asyncTimeoutTest("when client transport fallback, OnConnected Event in server not get called 2 times for same connection id", testUtilities.defaultTestTimeout * 8, function (end, assert, testName) {
    var connection = testUtilities.createHubConnection(end, assert, testName, undefined, false),
        savedProcessMessages = $.signalR.transports._logic.processMessages;
    var echoHub = connection.createHubProxies().echoHub,
        onConnectedEventCallCount = 0,
        onConnectedEventCallValue1;

    echoHub.client.echo = function (value) {
        if (value.indexOf("OnConnected") >1) {
            onConnectedEventCallCount++

            if (onConnectedEventCallCount === 1) {
                onConnectedEventCallValue1 = value;
            }

            if (onConnectedEventCallCount === 2) {
                if (onConnectedEventCallValue1 == value) {
                    assert.ok(false, "OnConnected Event in server called 2 times for same connection id");
                    end();
                }
            }
        }       
    };

    $.signalR.transports._logic.processMessages = function (_, minData) {
        // Look for initialize message, if we get it, ignore it, transports should time out
        if (connection.transport.name === "webSockets") {
            if (!minData.S) {
                savedProcessMessages.apply(this, arguments);
            }
        }
        else {
            savedProcessMessages.apply(this, arguments);
        }       
    }

    connection.start().done(function () {
        assert.ok(true, "Connection started");        
        end();
    }).fail(function () {
        // All transports fell back because they did not get init message, success!
        assert.comment("Connection failed to start!");
        end();
    });

    // Cleanup
    return function () {
        $.signalR.transports._logic.processMessages = savedProcessMessages;
        connection.stop();
    };
});

3). Run the test on IE10

Expected result:
OnConnected event in server get called 1 time for the connection id

Actual result:
we can see that OnConnected event in server get called 2 times for the same connection id

#2195

@halter73 halter73 added a commit that referenced this issue Sep 20, 2013
@halter73 halter73 Require the WebSocket Transport make an /abort request to abort
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
93d3bb8
@halter73 halter73 added a commit that referenced this issue Sep 20, 2013
@halter73 halter73 Require the WebSocket Transport make an /abort request to abort
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
a3bd526
@halter73 halter73 added a commit that referenced this issue Sep 20, 2013
@halter73 halter73 Require the WebSocket Transport make an /abort request to abort
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
98e0baa
@halter73 halter73 added a commit that referenced this issue Sep 23, 2013
@halter73 halter73 Require the WebSocket Transport make an /abort request to abort
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
718caae
@halter73 halter73 added a commit that referenced this issue Sep 25, 2013
@halter73 halter73 Require the WebSocket Transport make an /abort request to abort
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
28ce07c
@halter73 halter73 added a commit that referenced this issue Sep 25, 2013
@halter73 halter73 Require the WebSocket Transport make an /abort request to abort
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
1568fa6
@gustavo-armenta
Contributor

ran repro and onconnected is called once

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment