From 037e01b383382ec3b1a7a2c66f153e4b45559f34 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Fri, 19 Apr 2024 18:20:48 +0900 Subject: [PATCH] Keep track of reconnection atempts in delay calc Track how many times Sarus has already tried to (re)connect. When exponential backoff is enabled, use the stored number of connection attempts to calculate the exponential delay. --- __tests__/index/retryConnectionDelay.test.ts | 73 +++++++++++++++----- __tests__/index/state.test.ts | 25 +++++-- src/index.ts | 54 +++++++++++++-- 3 files changed, 125 insertions(+), 27 deletions(-) diff --git a/__tests__/index/retryConnectionDelay.test.ts b/__tests__/index/retryConnectionDelay.test.ts index c483236..bfb7c62 100644 --- a/__tests__/index/retryConnectionDelay.test.ts +++ b/__tests__/index/retryConnectionDelay.test.ts @@ -65,7 +65,7 @@ describe("retry connection delay", () => { }); describe("Exponential backoff delay", () => { - it("will never be more than 8000 ms with rate set to 2", () => { + describe("with rate 2, backoffLimit 8000 ms", () => { // The initial delay shall be 1 s const initialDelay = 1000; const exponentialBackoffParams: ExponentialBackoffParams = { @@ -73,20 +73,61 @@ describe("Exponential backoff delay", () => { // We put the ceiling at exactly 8000 ms backoffLimit: 8000, }; - expect( - calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 0), - ).toBe(1000); - expect( - calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 1), - ).toBe(2000); - expect( - calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 2), - ).toBe(4000); - expect( - calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 3), - ).toBe(8000); - expect( - calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 4), - ).toBe(8000); + it("will never be more than 8000 ms with rate set to 2", () => { + expect( + calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 0), + ).toBe(1000); + expect( + calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 1), + ).toBe(2000); + expect( + calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 2), + ).toBe(4000); + expect( + calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 3), + ).toBe(8000); + expect( + calculateRetryDelayFactor(exponentialBackoffParams, initialDelay, 4), + ).toBe(8000); + }); + + it("should delay reconnection attempts exponentially", async () => { + let setTimeoutCallback: any = undefined; + // The mocking here is a bit elaborate. We want to test the state machine + // in isolation. + const wsMock: any = {}; + (global as any).WebSocket = () => wsMock; + (global as any).setTimeout = (fn: () => void, delay: number) => { + setTimeoutCallback = fn; + }; + const sarus = new Sarus({ + url, + exponentialBackoff: exponentialBackoffParams, + }); + expect(sarus.state.kind).toBe("connecting"); + expect(wsMock.onopen).toBeTruthy(); + wsMock.onopen(); + expect(sarus.state.kind).toBe("connected"); + expect(setTimeoutCallback).toBeUndefined(); + wsMock.onclose(); + expect(sarus.state).toStrictEqual({ + kind: "closed", + failedConnectionAttempts: 0, + }); + expect(setTimeoutCallback).not.toBeUndefined(); + // expect ... to be called with N ms + if (setTimeoutCallback) { + setTimeoutCallback(); + } + expect(sarus.state).toStrictEqual({ + kind: "connecting", + failedConnectionAttempts: 0, + }); + wsMock.onclose(); + expect(sarus.state).toStrictEqual({ + kind: "connecting", + failedConnectionAttempts: 1, + }); + }); }); }); diff --git a/__tests__/index/state.test.ts b/__tests__/index/state.test.ts index 6900803..a586526 100644 --- a/__tests__/index/state.test.ts +++ b/__tests__/index/state.test.ts @@ -15,7 +15,12 @@ describe("state machine", () => { // In the beginning, the state is "connecting" const sarus: Sarus = new Sarus(sarusConfig); - expect(sarus.state.kind).toBe("connecting"); + // Since Sarus jumps into connecting directly, 1 connection attempt is made + // right in the beginning, but none have failed + expect(sarus.state).toStrictEqual({ + kind: "connecting", + failedConnectionAttempts: 0, + }); // We wait until we are connected, and see a "connected" state await server.connected; @@ -24,14 +29,22 @@ describe("state machine", () => { // When the connection drops, the state will be "closed" server.close(); await server.closed; - expect(sarus.state.kind).toBe("closed"); - - // Restart server - server = new WS(url); + expect(sarus.state).toStrictEqual({ + kind: "closed", + failedConnectionAttempts: 0, + }); // We wait a while, and the status is "connecting" again await delay(1); - expect(sarus.state.kind).toBe("connecting"); + // In the beginning, no connection attempts have been made, since in the + // case of a closed connection, we wait a bit until we try to connect again. + expect(sarus.state).toStrictEqual({ + kind: "connecting", + failedConnectionAttempts: 0, + }); + + // We restart the server and let the Sarus instance reconnect: + server = new WS(url); // When we connect in our mock server, we are "connected" again await server.connected; diff --git a/src/index.ts b/src/index.ts index c0cc69a..032806e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -180,10 +180,16 @@ export default class Sarus { * after the constructor wraps up. */ state: - | { kind: "connecting" } + | { kind: "connecting"; failedConnectionAttempts: number } | { kind: "connected" } | { kind: "disconnected" } - | { kind: "closed" } = { kind: "connecting" }; + /** + * The closed state carries of the number of failed connection attempts + */ + | { kind: "closed"; failedConnectionAttempts: number } = { + kind: "connecting", + failedConnectionAttempts: 0, + }; constructor(props: SarusClassParams) { // Extract the properties that are passed to the class @@ -378,7 +384,19 @@ export default class Sarus { * Connects the WebSocket client, and attaches event listeners */ connect() { - this.state = { kind: "connecting" }; + if (this.state.kind === "closed") { + this.state = { + kind: "connecting", + failedConnectionAttempts: this.state.failedConnectionAttempts, + }; + } else if ( + this.state.kind === "connected" || + this.state.kind === "disconnected" + ) { + this.state = { kind: "connecting", failedConnectionAttempts: 0 }; + } else { + // This is a NOOP, we are already connecting + } this.ws = new WebSocket(this.url, this.protocols); this.setBinaryType(); this.attachEventListeners(); @@ -392,11 +410,22 @@ export default class Sarus { reconnect() { const self = this; const { retryConnectionDelay, exponentialBackoff } = self; + // If we are already in a "connecting" state, we need to refer to the + // current amount of connection attemps to correctly calculate the + // exponential delay -- if exponential backoff is enabled. + const failedConnectionAttempts = + self.state.kind === "connecting" + ? self.state.failedConnectionAttempts + : 0; // If no exponential backoff is enabled, retryConnectionDelay will // be scaled by a factor of 1 and it will stay the original value. const delay = exponentialBackoff - ? calculateRetryDelayFactor(exponentialBackoff, retryConnectionDelay, 0) + ? calculateRetryDelayFactor( + exponentialBackoff, + retryConnectionDelay, + failedConnectionAttempts, + ) : retryConnectionDelay; setTimeout(self.connect, delay); @@ -544,7 +573,22 @@ export default class Sarus { if (eventName === "open") { self.state = { kind: "connected" }; } else if (eventName === "close" && self.reconnectAutomatically) { - self.state = { kind: "closed" }; + const { state } = self; + // If we have previously been "connecting", we carry over the amount + // of failed connection attempts and add 1, since the current + // connection attempt failed. We stay "connecting" instead of + // "closed", since we've never been fully "connected" in the first + // place. + if (state.kind === "connecting") { + self.state = { + kind: "connecting", + failedConnectionAttempts: state.failedConnectionAttempts + 1, + }; + } else { + // If we were in a different state, we assume that our connection + // freshly closed and have not made any failed connection attempts. + self.state = { kind: "closed", failedConnectionAttempts: 0 }; + } self.removeEventListeners(); self.reconnect(); }