pladaria / reconnecting-websocket

Reconnecting WebSocket. For Web, React Native, cli (Node.js)
MIT License
1.23k stars 199 forks source link

Fix 'open' event listener overriding previously initialized reconnectDelay #25

Closed lukes closed 6 years ago

lukes commented 7 years ago

Previously the updateReconnectionDelay function was always being overridden by initReconnectionDelay within the 'open' event listener when the connection was being disconnected.

This meant that the reconnectionDelayGrowFactor functionality was not working, and the reconnection attempt would run according to the logic in initReconnectionDelay only.

To test, connect to a websocket using broken auth (or have the websocket server always close the connection some other way), using these options:

url = 'wss://websocket.com/your-uri';

const options = {
  debug: true,
  minReconnectionDelay: 500,
  maxReconnectionDelay: 600000,
  maxRetries: Infinity,
  reconnectionDelayGrowFactor: 1.3,
};

new ReconnectingWebSocket(url, undefined, options);

Previous behavior was for reconnectionDelayGrowFactor to be ignored.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 1d8d253f80ffbd7733c4a48816f5a6842db1ccdf on lukes:lukes-patch-1 into e79fba552f236463adac04e1872f5e20e37d66a1 on pladaria:master.

pladaria commented 7 years ago

This behavior was intentional. After a successful connect, the reconnectionDelay is reset. But you are absolutely right, this is a problem when the server closes the connection fast for any reason.

As an improvement I could add another setting, let's call it minUptimeBeforeReconnectionDelayReset. If the connection is closed before this interval the delay is not reset.

What do you think?

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 87943817319ae54f6ea78f8a75fc78a39273a8f4 on lukes:lukes-patch-1 into e79fba552f236463adac04e1872f5e20e37d66a1 on pladaria:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 87943817319ae54f6ea78f8a75fc78a39273a8f4 on lukes:lukes-patch-1 into e79fba552f236463adac04e1872f5e20e37d66a1 on pladaria:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 1f1dbd5e8d474eb0c0996ff71d9222e0a2c45565 on lukes:lukes-patch-1 into e79fba552f236463adac04e1872f5e20e37d66a1 on pladaria:master.

lukes commented 7 years ago

Hi, yes you're right, the reconnectDelay was never reset.

...this is a problem when the server closes the connection fast for any reason

It happens even when the client has been connected to the server for a long time too, i.e., the websocket server is shut down.

I don't understand every feature your code is supporting, but I've pushed a change which fixes the issue, and resets the reconnectDelay with every disconnection.

Here's a console.log session:

# Client connects:

RWS: init
RWS: connect
RWS: bypass properties
RWS: open
RWS: reconnectDelay: 880.3215349622476
RWS: handleClose {shouldRetry: true}

# Server shuts down, reconnectDelay increments correctly

RWS: retries count: 1
RWS: handleClose - reconnectDelay: 1144.4179954509218
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 2
RWS: handleClose - reconnectDelay: 1487.7433940861984
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 3
RWS: handleClose - reconnectDelay: 1934.066412312058
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 4
RWS: handleClose - reconnectDelay: 2514.2863360056754
RWS: timeout
RWS: timeout
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 5
RWS: handleClose - reconnectDelay: 3268.572236807378
RWS: timeout
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 6
RWS: handleClose - reconnectDelay: 4249.143907849591
RWS: timeout
RWS: timeout
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 7
RWS: handleClose - reconnectDelay: 5523.887080204469
RWS: timeout
RWS: connect
RWS: bypass properties

# Server comes back, and client reconnects reconnectDelay is reset

RWS: open
RWS: reconnectDelay: 670.5218059215576

# Server shuts down again, reconnectDelay increments correctly

RWS: handleClose {shouldRetry: true}
RWS: retries count: 1
RWS: handleClose - reconnectDelay: 871.6783476980249
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 2
RWS: handleClose - reconnectDelay: 1133.1818520074326
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 3
RWS: handleClose - reconnectDelay: 1473.1364076096625
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 4
RWS: handleClose - reconnectDelay: 1915.0773298925612
RWS: timeout
RWS: connect
RWS: bypass properties
RWS: handleClose {shouldRetry: true}
RWS: retries count: 5
RWS: handleClose - reconnectDelay: 2489.6005288603296
RWS: timeout
RWS: timeout
RWS: connect
pladaria commented 6 years ago

I'v just release a complete rewrite of the library. It now handles this differently

lukes commented 6 years ago

Awesome work!