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

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

Closed
Xiaohongt opened this issue Jun 23, 2013 · 2 comments
Assignees
Milestone

Comments

@Xiaohongt
Copy link
Contributor

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

@ghost ghost assigned halter73 Jun 29, 2013
@davidfowl
Copy link
Member

@halter73 this is a bug with the websocket transport aggressively removing connections when the websocket is closed.

halter73 added a commit that referenced this issue Jul 3, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231
halter73 added a commit that referenced this issue Jul 3, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231
@ghost ghost assigned Xiaohongt Jul 4, 2013
@Xiaohongt
Copy link
Contributor Author

verified that OnConnected event in server called one time

halter73 added a commit that referenced this issue Sep 20, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
halter73 added a commit that referenced this issue Sep 20, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
halter73 added a commit that referenced this issue Sep 20, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
halter73 added a commit that referenced this issue Sep 23, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
halter73 added a commit that referenced this issue Sep 25, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
halter73 added a commit that referenced this issue Sep 25, 2013
- Avoids timeouts being mistakenly registered as aborts
- Unifies abort logic for all transports

#2195 #1231

#2500
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants