Modified configurePingInterval to check if it is already set #2772
Conversation
| @@ -101,7 +101,7 @@ | |||
| $(connection).triggerHandler(events.onError, [error]); | |||
| }; | |||
|
|
|||
| if (!config.pingIntervalId && config.pingInterval) { | |||
| if (!connection._.config && config.pingInterval) { | |||
There was a problem hiding this comment.
Shouldn't this be if (!connection._.pingIntervalId && config.pingInterval) { like Taylor wrote in the issue? #2739
|
You should add a unit test for this ensuring the pingIntervalId doesn't change. |
|
I need this change yesterday. |
|
|
|
I tried this fix on jabbr and it still fails when the connection is stopped and restarted with the following: Seems like the connection._.config property is undefined. |
|
Ok this is an EPIC bug @NTaylorMullen thanks for the christmas gift When the connection is restarted, connection._.lastActiveAt isn't cleared, this causes restarted connections to run this logic on startup: Which in turns calls stop() and deletes all connection state. Here's what the log in jabbr looks: |
. So there's 2 issues, the one that 
|
Patched in jabbr here JabbR/JabbR@101c9b6. Will open an issue for this in 2.0.2. |
| @@ -95,19 +95,6 @@ | |||
| return connection.state === signalR.connectionState.disconnected; | |||
| }, | |||
|
|
|||
| configurePingInterval = function (connection) { | |||
There was a problem hiding this comment.
Why did we move this? Testing?
|
@davidfowl any more changes needed here? |
#2739