From 2aa5c842ddcb26c7a1342bab789ffb9f76f19ea3 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Fri, 19 Apr 2024 17:39:26 +0900 Subject: [PATCH 1/7] Change state to be a tagged-union like type By turning state from simple string literals to full objects, we can add annotations that are specific to one state. Use case: When retrying connections, we could just add a connectionAttempt?: number; property, but then we would have to check if it is undefined or not, and if we are not reconnecting currently and connectionAttempt? is set, does that mean the Sarus client entered an invalid state? By guaranteeing certain properties to only ever be present when the state property has the right kind, we can avoid a whole class of bugs just by relying on the type system. --- __tests__/index/state.test.ts | 20 ++++++++++---------- src/index.ts | 29 ++++++++++++++++++++++------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/__tests__/index/state.test.ts b/__tests__/index/state.test.ts index 470868c..6900803 100644 --- a/__tests__/index/state.test.ts +++ b/__tests__/index/state.test.ts @@ -15,27 +15,27 @@ describe("state machine", () => { // In the beginning, the state is "connecting" const sarus: Sarus = new Sarus(sarusConfig); - expect(sarus.state).toBe("connecting"); + expect(sarus.state.kind).toBe("connecting"); // We wait until we are connected, and see a "connected" state await server.connected; - expect(sarus.state).toBe("connected"); + expect(sarus.state.kind).toBe("connected"); // When the connection drops, the state will be "closed" server.close(); await server.closed; - expect(sarus.state).toBe("closed"); + expect(sarus.state.kind).toBe("closed"); // Restart server server = new WS(url); // We wait a while, and the status is "connecting" again await delay(1); - expect(sarus.state).toBe("connecting"); + expect(sarus.state.kind).toBe("connecting"); // When we connect in our mock server, we are "connected" again await server.connected; - expect(sarus.state).toBe("connected"); + expect(sarus.state.kind).toBe("connected"); // Cleanup server.close(); @@ -46,23 +46,23 @@ describe("state machine", () => { // Same initial state transition as above const sarus: Sarus = new Sarus(sarusConfig); - expect(sarus.state).toBe("connecting"); + expect(sarus.state.kind).toBe("connecting"); await server.connected; - expect(sarus.state).toBe("connected"); + expect(sarus.state.kind).toBe("connected"); // The user can disconnect and the state will be "disconnected" sarus.disconnect(); - expect(sarus.state).toBe("disconnected"); + expect(sarus.state.kind).toBe("disconnected"); await server.closed; // The user can now reconnect, and the state will be "connecting", and then // "connected" again sarus.connect(); - expect(sarus.state).toBe("connecting"); + expect(sarus.state.kind).toBe("connecting"); await server.connected; // XXX for some reason the test will fail without waiting 10 ms here await delay(10); - expect(sarus.state).toBe("connected"); + expect(sarus.state.kind).toBe("connected"); server.close(); }); }); diff --git a/src/index.ts b/src/index.ts index 269d75c..d7dfbd0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -134,10 +134,25 @@ export default class Sarus { * * connect(), disconnect() are generally called by the user * - * this.reconnect() is called internally when automatic reconnection is - * enabled, but can also be called by the user + * When disconnected by the WebSocket itself (i.e., this.ws.onclose), + * this.reconnect() is called automatically if reconnection is enabled. + * this.reconnect() can also be called by the user, for example if + * this.disconnect() was purposefully called and reconnection is desired. + * + * The current state is specified by the 'kind' property of state + * Each state can have additional data contained in properties other than + * 'kind'. Those properties might be unique to one state, or contained in + * several states. To access a property, it might be necessary to narrow down + * the 'kind' of state. + * + * The initial state is connecting, as a Sarus client tries to connect right + * after the constructor wraps up. */ - state: "connecting" | "connected" | "disconnected" | "closed" = "connecting"; + state: + | { kind: "connecting" } + | { kind: "connected" } + | { kind: "disconnected" } + | { kind: "closed" } = { kind: "connecting" }; constructor(props: SarusClassParams) { // Extract the properties that are passed to the class @@ -324,7 +339,7 @@ export default class Sarus { * Connects the WebSocket client, and attaches event listeners */ connect() { - this.state = "connecting"; + this.state = { kind: "connecting" }; this.ws = new WebSocket(this.url, this.protocols); this.setBinaryType(); this.attachEventListeners(); @@ -347,7 +362,7 @@ export default class Sarus { * @param {boolean} overrideDisableReconnect */ disconnect(overrideDisableReconnect?: boolean) { - this.state = "disconnected"; + this.state = { kind: "disconnected" }; const self = this; // We do this to prevent automatic reconnections; if (!overrideDisableReconnect) { @@ -480,9 +495,9 @@ export default class Sarus { self.ws[`on${eventName}`] = (e: Function) => { self.eventListeners[eventName].forEach((f: Function) => f(e)); if (eventName === "open") { - self.state = "connected"; + self.state = { kind: "connected" }; } else if (eventName === "close" && self.reconnectAutomatically) { - self.state = "closed"; + self.state = { kind: "closed" }; self.removeEventListeners(); self.reconnect(); } From 6a7d7f290d0c8c4adbbd48c628f28d860215f5f9 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Fri, 19 Apr 2024 19:36:05 +0900 Subject: [PATCH 2/7] Narrow down websocket on* event type Previously, it said "Function", but this was probably meant in reference to the callbacks called further below. Refering to the MDN docs on WebSocket Interfaces [^1], one can see that there are three possible events: - Event, - CloseEvent, and - MessageEvent [^1][https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API#interfaces] --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index d7dfbd0..41186de 100644 --- a/src/index.ts +++ b/src/index.ts @@ -492,7 +492,7 @@ export default class Sarus { attachEventListeners() { const self: any = this; WS_EVENT_NAMES.forEach((eventName) => { - self.ws[`on${eventName}`] = (e: Function) => { + self.ws[`on${eventName}`] = (e: Event | CloseEvent | MessageEvent) => { self.eventListeners[eventName].forEach((f: Function) => f(e)); if (eventName === "open") { self.state = { kind: "connected" }; From 1bf3f85629c59d919208bcd6e31b22c439402a33 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Tue, 2 Jan 2024 09:36:01 +0900 Subject: [PATCH 3/7] Add optional exponential backoff config Allow configuring and enabling exponential backoff for failing connections, instead of relying on reconnections in regular intervals. Exponential backoff can be configured in order to lighten server load if multiple clients are reconnecting. The new configuration parameter is called `exponentialBackoff` and requires two parameters `backoffRate` and `backoffLimit`. --- README.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/index.ts | 18 ++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4c8a28b..7c7bee6 100644 --- a/README.md +++ b/README.md @@ -352,6 +352,68 @@ If you think that there is a potential case for you ending up queuing at least wrap `sarus.send` function calls in a try/catch statement, so as to handle those messages, should they occur. +### Exponential backoff + +Configure exponential backoff like so: + +```typescript +import Sarus from '@anephenix/sarus'; + +const sarus = new Sarus({ + url: 'wss://ws.anephenix.com', + exponentialBackoff: { + // Exponential factor, here 2 will result in + // 1 s, 2 s, 4 s, and so on increasing delays + backoffRate: 2, + // Never wait more than 2000 seconds + backoffLimit: 2000, + }, +}); +``` + +When a connection attempt repeatedly fails, decreasing the delay +exponentially between each subsequent reconnection attempt is called +[Exponential backoff](https://en.wikipedia.org/wiki/Exponential_backoff). The +idea is that if a connection attempt failed after 1 second, and 2 seconds, then it is +not necessary to check it on the 3rd second, since the probability of a +reconnection succeeding on the third attempt is most likely not going up. +Therefore, increasing the delay between each attempt factors in the assumption +that a connection is not more likely to succeed by repeatedly probing in regular +intervals. + +This decreases both the load on the client, as well as on the server. For +a client, fewer websocket connection attempts decrease the load on the client +and on the network connection. For the server, should websocket requests fail +within, then the load for handling repeatedly failing requests will fall +as well. Furthermore, the burden on the network will also be decreased. Should +for example a server refuse to accept websocket connections for one client, +then there is the possibility that other clients will also not be able to connect. + +Sarus implements _truncated exponential backoff_, meaning that the maximum +reconnection delay is capped by another factor `backoffLimit` and will never +exceed it. The exponential backoff rate itself is determined by `backoffRate`. +If `backoffRate` is 2, then the delays will be 1 s, 2 s, 4 s, and so on. + +The algorithm for reconnection looks like this in pseudocode: + +```javascript +// Configurable +const backoffRate = 2; +// The maximum delay will be 400s +const backoffLimit = 400; +let notConnected = false; +let connectionAttempts = 1; +while (notConnected) { + const delay = Math.min( + Math.pow(connectionAttempts, backoffRate), + backoffLimit, + ); + await delay(delay); + notConnected = tryToConnect(); + connectionAttempts += 1; +} +``` + ### Advanced options Sarus has a number of other options that you can pass to the client during diff --git a/src/index.ts b/src/index.ts index 41186de..d89639a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -73,6 +73,11 @@ const validateWebSocketUrl = (rawUrl: string): URL => { return url; }; +export interface ExponentialBackoffParams { + backoffRate: number; + backoffLimit: number; +} + export interface SarusClassParams { url: string; binaryType?: BinaryType; @@ -81,6 +86,7 @@ export interface SarusClassParams { retryProcessTimePeriod?: number; reconnectAutomatically?: boolean; retryConnectionDelay?: boolean | number; + exponentialBackoff?: ExponentialBackoffParams; storageType?: string; storageKey?: string; } @@ -95,7 +101,8 @@ export interface SarusClassParams { * @param {object} param0.eventListeners - An optional object containing event listener functions keyed to websocket events * @param {boolean} param0.reconnectAutomatically - An optional boolean flag to indicate whether to reconnect automatically when a websocket connection is severed * @param {number} param0.retryProcessTimePeriod - An optional number for how long the time period between retrying to send a messgae to a WebSocket server should be - * @param {number} param0.retryConnectionDelay - A parameter for the amount of time to delay a reconnection attempt by, in miliseconds. + * @param {boolean|number} param0.retryConnectionDelay - An optional parameter for whether to delay WebSocket reconnection attempts by a time period. If true, the delay is 1000ms, otherwise it is the number passed. The default value when this parameter is undefined will be interpreted as 1000ms. + * @param {ExponentialBackoffParams} param0.exponentialBackoff - An optional containing configuration for exponential backoff. If this parameter is undefined, exponential backoff is disabled. The minimum delay is determined by retryConnectionDelay. If retryConnectionDelay is set is false, this setting will not be in effect. * @param {string} param0.storageType - An optional string specifying the type of storage to use for persisting messages in the message queue * @param {string} param0.storageKey - An optional string specifying the key used to store the messages data against in sessionStorage/localStorage * @returns {object} The class instance @@ -109,6 +116,7 @@ export default class Sarus { retryProcessTimePeriod?: number; reconnectAutomatically?: boolean; retryConnectionDelay: number; + exponentialBackoff?: ExponentialBackoffParams; storageType: string; storageKey: string; @@ -164,6 +172,7 @@ export default class Sarus { reconnectAutomatically, retryProcessTimePeriod, // TODO - write a test case to check this retryConnectionDelay, + exponentialBackoff, storageType = "memory", storageKey = "sarus", } = props; @@ -208,6 +217,13 @@ export default class Sarus { ? undefined : retryConnectionDelay) ?? 1000; + /* + When a exponential backoff parameter object is provided, reconnection + attemptions will be increasingly delayed by an exponential factor. + This feature is disabled by default. + */ + this.exponentialBackoff = exponentialBackoff; + /* Sets the storage type for the messages in the message queue. By default it is an in-memory option, but can also be set as 'session' for From 892a5f9ccf472b8fcabba780f6d05af599c54465 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Fri, 19 Apr 2024 17:52:44 +0900 Subject: [PATCH 4/7] Add calculateRetryDelayFactor helper function This is used to calculate the exponentially increasing delay. The function is used to calculate the reconnection delay - but does not keep track of the current reconnection attempt. Keeping track of the current reconnection attempt is reserved for the next few commits. --- __tests__/index/retryConnectionDelay.test.ts | 29 ++++++++++++++++ src/index.ts | 36 ++++++++++++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/__tests__/index/retryConnectionDelay.test.ts b/__tests__/index/retryConnectionDelay.test.ts index a0c4376..c483236 100644 --- a/__tests__/index/retryConnectionDelay.test.ts +++ b/__tests__/index/retryConnectionDelay.test.ts @@ -1,6 +1,8 @@ // File Dependencies import Sarus from "../../src/index"; import { WS } from "jest-websocket-mock"; +import { calculateRetryDelayFactor } from "../../src/index"; +import type { ExponentialBackoffParams } from "../../src/index"; const url = "ws://localhost:1234"; @@ -61,3 +63,30 @@ describe("retry connection delay", () => { }); }); }); + +describe("Exponential backoff delay", () => { + it("will never be more than 8000 ms with rate set to 2", () => { + // The initial delay shall be 1 s + const initialDelay = 1000; + const exponentialBackoffParams: ExponentialBackoffParams = { + backoffRate: 2, + // 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); + }); +}); diff --git a/src/index.ts b/src/index.ts index d89639a..be44aa4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -78,6 +78,28 @@ export interface ExponentialBackoffParams { backoffLimit: number; } +/* + * Calculate the exponential backoff delay for a given number of connection + * attempts. + * @param {ExponentialBackoffParams} params - configuration parameters for + * exponential backoff. + * @param {number} initialDelay - the initial delay before any backoff is + * applied + * @param {number} failedConnectionAttempts - the number of connection attempts + * that have previously failed + * @returns {void} - set does not return + */ +export function calculateRetryDelayFactor( + params: ExponentialBackoffParams, + initialDelay: number, + failedConnectionAttempts: number, +): number { + return Math.min( + initialDelay * Math.pow(params.backoffRate, failedConnectionAttempts), + params.backoffLimit, + ); +} + export interface SarusClassParams { url: string; binaryType?: BinaryType; @@ -363,12 +385,20 @@ export default class Sarus { } /** - * Reconnects the WebSocket client based on the retryConnectionDelay setting. + * Reconnects the WebSocket client based on the retryConnectionDelay and + * ExponentialBackoffParam setting. */ reconnect() { const self = this; - const { retryConnectionDelay } = self; - setTimeout(self.connect, retryConnectionDelay); + const { retryConnectionDelay, exponentialBackoff } = self; + + // 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) + : retryConnectionDelay; + + setTimeout(self.connect, delay); } /** From a59058f789c84823757918f255e44c8adff24e42 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Fri, 19 Apr 2024 18:20:48 +0900 Subject: [PATCH 5/7] 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 | 89 ++++++++++++++++---- __tests__/index/state.test.ts | 25 ++++-- src/index.ts | 54 ++++++++++-- 3 files changed, 141 insertions(+), 27 deletions(-) diff --git a/__tests__/index/retryConnectionDelay.test.ts b/__tests__/index/retryConnectionDelay.test.ts index c483236..f343342 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,77 @@ 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 () => { + const webSocketSpy = jest.spyOn(global, "WebSocket" as any); + webSocketSpy.mockImplementation(() => {}); + const setTimeoutSpy = jest.spyOn(global, "setTimeout"); + const sarus = new Sarus({ + url, + exponentialBackoff: exponentialBackoffParams, + }); + expect(sarus.state).toStrictEqual({ + kind: "connecting", + failedConnectionAttempts: 0, + }); + let instance: WebSocket; + [instance] = webSocketSpy.mock.instances; + if (!instance.onopen) { + throw new Error(); + } + instance.onopen(new Event("open")); + if (!instance.onclose) { + throw new Error(); + } + instance.onclose(new CloseEvent("close")); + + let cb: Sarus["connect"]; + // We iteratively call sarus.connect() and let it fail, seeing + // if it reaches 8000 as a delay and stays there + const attempts: [number, number][] = [ + [1000, 1], + [2000, 2], + [4000, 3], + [8000, 4], + [8000, 5], + ]; + attempts.forEach(([delay, failedAttempts]: [number, number]) => { + const call = + setTimeoutSpy.mock.calls[setTimeoutSpy.mock.calls.length - 1]; + if (!call) { + throw new Error(); + } + expect(call[1]).toBe(delay); + cb = call[0]; + cb(); + instance = + webSocketSpy.mock.instances[webSocketSpy.mock.instances.length - 1]; + if (!instance.onclose) { + throw new Error(); + } + instance.onclose(new CloseEvent("close")); + expect(sarus.state).toStrictEqual({ + kind: "connecting", + failedConnectionAttempts: failedAttempts, + }); + }); + }); }); }); 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 be44aa4..7ab0b4d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -179,10 +179,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 @@ -377,7 +383,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(); @@ -391,11 +409,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); @@ -543,7 +572,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(); } From 2aedd088306d42596760900359e12099f9dcc0b6 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Sat, 20 Apr 2024 18:29:31 +0900 Subject: [PATCH 6/7] Simplify exponential backoff test cases --- __tests__/index/retryConnectionDelay.test.ts | 61 ++++++++++---------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/__tests__/index/retryConnectionDelay.test.ts b/__tests__/index/retryConnectionDelay.test.ts index f343342..c8c5cbe 100644 --- a/__tests__/index/retryConnectionDelay.test.ts +++ b/__tests__/index/retryConnectionDelay.test.ts @@ -68,71 +68,74 @@ describe("Exponential backoff delay", () => { describe("with rate 2, backoffLimit 8000 ms", () => { // The initial delay shall be 1 s const initialDelay = 1000; - const exponentialBackoffParams: ExponentialBackoffParams = { + const exponentialBackoff: ExponentialBackoffParams = { backoffRate: 2, // We put the ceiling at exactly 8000 ms backoffLimit: 8000, }; + const attempts: [number, number][] = [ + [1000, 0], + [2000, 1], + [4000, 2], + [8000, 3], + [8000, 4], + ]; 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); + attempts.forEach(([delay, failedAttempts]) => { + expect( + calculateRetryDelayFactor( + exponentialBackoff, + initialDelay, + failedAttempts, + ), + ).toBe(delay); + }); }); it("should delay reconnection attempts exponentially", async () => { + // Somehow we need to convincen typescript here that "WebSocket" is + // totally valid. Could be because it doesn't assume WebSocket is part of + // global / the index key is missing const webSocketSpy = jest.spyOn(global, "WebSocket" as any); webSocketSpy.mockImplementation(() => {}); const setTimeoutSpy = jest.spyOn(global, "setTimeout"); - const sarus = new Sarus({ - url, - exponentialBackoff: exponentialBackoffParams, - }); + const sarus = new Sarus({ url, exponentialBackoff }); expect(sarus.state).toStrictEqual({ kind: "connecting", failedConnectionAttempts: 0, }); let instance: WebSocket; + // Get the first WebSocket instance, and ... [instance] = webSocketSpy.mock.instances; if (!instance.onopen) { throw new Error(); } + // tell the sarus instance that it is open, and ... instance.onopen(new Event("open")); if (!instance.onclose) { throw new Error(); } + // close it immediately instance.onclose(new CloseEvent("close")); + expect(sarus.state).toStrictEqual({ + kind: "closed", + failedConnectionAttempts: 0, + }); let cb: Sarus["connect"]; // We iteratively call sarus.connect() and let it fail, seeing // if it reaches 8000 as a delay and stays there - const attempts: [number, number][] = [ - [1000, 1], - [2000, 2], - [4000, 3], - [8000, 4], - [8000, 5], - ]; - attempts.forEach(([delay, failedAttempts]: [number, number]) => { + attempts.forEach(([delay, failedAttempts]) => { const call = setTimeoutSpy.mock.calls[setTimeoutSpy.mock.calls.length - 1]; if (!call) { throw new Error(); } + // Make sure that setTimeout was called with the correct delay expect(call[1]).toBe(delay); cb = call[0]; cb(); + // Get the most recent WebSocket instance instance = webSocketSpy.mock.instances[webSocketSpy.mock.instances.length - 1]; if (!instance.onclose) { @@ -141,7 +144,7 @@ describe("Exponential backoff delay", () => { instance.onclose(new CloseEvent("close")); expect(sarus.state).toStrictEqual({ kind: "connecting", - failedConnectionAttempts: failedAttempts, + failedConnectionAttempts: failedAttempts + 1, }); }); }); From 6e88d537a358ea17fcc1bd732ef99484b732f855 Mon Sep 17 00:00:00 2001 From: Justus Perlwitz Date: Sat, 20 Apr 2024 18:54:08 +0900 Subject: [PATCH 7/7] Remove failed conn. attempts from closed state It turns out we don't need it here after all --- __tests__/index/retryConnectionDelay.test.ts | 1 - __tests__/index/state.test.ts | 1 - src/index.ts | 20 ++++---------------- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/__tests__/index/retryConnectionDelay.test.ts b/__tests__/index/retryConnectionDelay.test.ts index c8c5cbe..6b5b383 100644 --- a/__tests__/index/retryConnectionDelay.test.ts +++ b/__tests__/index/retryConnectionDelay.test.ts @@ -119,7 +119,6 @@ describe("Exponential backoff delay", () => { instance.onclose(new CloseEvent("close")); expect(sarus.state).toStrictEqual({ kind: "closed", - failedConnectionAttempts: 0, }); let cb: Sarus["connect"]; diff --git a/__tests__/index/state.test.ts b/__tests__/index/state.test.ts index a586526..24ef7db 100644 --- a/__tests__/index/state.test.ts +++ b/__tests__/index/state.test.ts @@ -31,7 +31,6 @@ describe("state machine", () => { await server.closed; expect(sarus.state).toStrictEqual({ kind: "closed", - failedConnectionAttempts: 0, }); // We wait a while, and the status is "connecting" again diff --git a/src/index.ts b/src/index.ts index 7ab0b4d..edb8616 100644 --- a/src/index.ts +++ b/src/index.ts @@ -182,10 +182,7 @@ export default class Sarus { | { kind: "connecting"; failedConnectionAttempts: number } | { kind: "connected" } | { kind: "disconnected" } - /** - * The closed state carries of the number of failed connection attempts - */ - | { kind: "closed"; failedConnectionAttempts: number } = { + | { kind: "closed" } = { kind: "connecting", failedConnectionAttempts: 0, }; @@ -383,18 +380,9 @@ export default class Sarus { * Connects the WebSocket client, and attaches event listeners */ connect() { - if (this.state.kind === "closed") { - this.state = { - kind: "connecting", - failedConnectionAttempts: this.state.failedConnectionAttempts, - }; - } else if ( - this.state.kind === "connected" || - this.state.kind === "disconnected" - ) { + // If we aren't already connecting, we are now + if (this.state.kind !== "connecting") { 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(); @@ -586,7 +574,7 @@ export default class Sarus { } 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.state = { kind: "closed" }; } self.removeEventListeners(); self.reconnect();