From 5d7daaaaf56c76c4a38913d06952e22d1aa80dbe Mon Sep 17 00:00:00 2001 From: leibale Date: Mon, 1 Aug 2022 20:16:51 -0400 Subject: [PATCH 1/6] handle errors in reconnect strategy --- packages/client/lib/client/socket.ts | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 5fab5c5601b..7b98aa0b8d7 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -5,11 +5,13 @@ import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; +type ReconnectStrategy = (retires: number) => number | Error; + export interface RedisSocketCommonOptions { connectTimeout?: number; noDelay?: boolean; keepAlive?: number | false; - reconnectStrategy?(retries: number): number | Error; + reconnectStrategy?: ReconnectStrategy; } type RedisNetSocketOptions = Partial & { @@ -44,10 +46,6 @@ export default class RedisSocket extends EventEmitter { return options; } - static #defaultReconnectStrategy(retries: number): number { - return Math.min(retries * 50, 500); - } - static #isTlsSocket(options: RedisSocketOptions): options is RedisTlsSocketOptions { return (options as RedisTlsSocketOptions).tls === true; } @@ -87,6 +85,23 @@ export default class RedisSocket extends EventEmitter { this.#options = RedisSocket.#initiateOptions(options); } + reconnectStrategy(retries: number): number | Error { + if (this.#options.reconnectStrategy) { + try { + const retryIn = this.#options.reconnectStrategy(retries); + if (typeof retryIn !== 'number' && !(retryIn instanceof Error)) { + throw new TypeError('Reconnect strategy should return `number | Error`'); + } + + return retryIn; + } catch (err) { + this.emit('error', err); + } + } + + return Math.min(retries * 50, 500); + } + async connect(): Promise { if (this.#isOpen) { throw new Error('Socket already opened'); @@ -118,7 +133,7 @@ export default class RedisSocket extends EventEmitter { } catch (err) { this.emit('error', err); - const retryIn = (this.#options?.reconnectStrategy ?? RedisSocket.#defaultReconnectStrategy)(retries); + const retryIn = this.reconnectStrategy(retries); if (retryIn instanceof Error) { this.#isOpen = false; throw new ReconnectStrategyError(retryIn, err); From ce211924da713bc75782f06f423b06135f781821 Mon Sep 17 00:00:00 2001 From: leibale Date: Mon, 8 Aug 2022 08:04:04 -0400 Subject: [PATCH 2/6] add test --- packages/client/lib/client/socket.spec.ts | 56 ++++++++++++++++------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 54f84eb9fe0..1b2d050c012 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -1,41 +1,63 @@ import { strict as assert } from 'assert'; import { SinonFakeTimers, useFakeTimers, spy } from 'sinon'; -import RedisSocket from './socket'; +import RedisSocket, { RedisSocketOptions } from './socket'; describe('Socket', () => { + function createSocket(options: RedisSocketOptions): RedisSocket { + const socket = new RedisSocket( + () => Promise.resolve(), + options + ); + + socket.on('error', (err) => { + // ignore errors + console.log(err); + }); + + return socket; + } + describe('reconnectStrategy', () => { let clock: SinonFakeTimers; beforeEach(() => clock = useFakeTimers()); afterEach(() => clock.restore()); - it('custom strategy', () => { - const reconnectStrategy = spy((retries: number): number | Error => { + it('custom strategy', async () => { + const reconnectStrategy = spy((retries: number) => { assert.equal(retries + 1, reconnectStrategy.callCount); - if (retries === 50) { - return Error('50'); - } + if (retries === 50) return new Error('50'); const time = retries * 2; queueMicrotask(() => clock.tick(time)); return time; }); - const socket = new RedisSocket( - () => Promise.resolve(), - { - host: 'error', - reconnectStrategy - } - ); - - socket.on('error', () => { - // ignore errors + const socket = createSocket({ + host: 'error', + reconnectStrategy }); - return assert.rejects(socket.connect(), { + await assert.rejects(socket.connect(), { message: '50' }); + + assert.equal(socket.isOpen, false); + }); + + it('should handle errors', async () => { + const socket = createSocket({ + host: 'error', + reconnectStrategy(retries: number) { + if (retries === 1) return new Error('done'); + queueMicrotask(() => clock.tick(500)); + throw new Error(); + } + }); + + await assert.rejects(socket.connect()); + + assert.equal(socket.isOpen, false); }); }); }); From aa03a7854a7b0faac8e0558494681e228fe81905 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 18 Aug 2022 11:32:52 -0400 Subject: [PATCH 3/6] fix retries typo --- packages/client/lib/client/socket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 7b98aa0b8d7..abb795d7bad 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -5,7 +5,7 @@ import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; -type ReconnectStrategy = (retires: number) => number | Error; +type ReconnectStrategy = (retries: number) => number | Error; export interface RedisSocketCommonOptions { connectTimeout?: number; From c8d32e5531628a9d3e4ac93989f11badb6f4daaf Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 18 Aug 2022 12:11:33 -0400 Subject: [PATCH 4/6] fix #2237 - flush queues on reconnect strategy error --- packages/client/lib/client/socket.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 7b98aa0b8d7..767e95f6a63 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -131,14 +131,14 @@ export default class RedisSocket extends EventEmitter { this.#isReady = true; this.emit('ready'); } catch (err) { - this.emit('error', err); - const retryIn = this.reconnectStrategy(retries); if (retryIn instanceof Error) { this.#isOpen = false; + this.emit('error', err); throw new ReconnectStrategyError(retryIn, err); } + this.emit('error', err); await promiseTimeout(retryIn); return this.#connect(retries + 1); } From 52f0fb5b5351c088e7515e3d24526bab21e71cc3 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Mon, 22 Aug 2022 14:30:59 -0400 Subject: [PATCH 5/6] Update socket.ts --- packages/client/lib/client/socket.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index fdf758326b9..1bf012d7b28 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -5,13 +5,13 @@ import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; -type ReconnectStrategy = (retries: number) => number | Error; +type ReconnectStrategy = ; export interface RedisSocketCommonOptions { connectTimeout?: number; noDelay?: boolean; keepAlive?: number | false; - reconnectStrategy?: ReconnectStrategy; + reconnectStrategy?(retries: number): number | Error; } type RedisNetSocketOptions = Partial & { From 18bf77aa94bb8512c2d23d7aaa0096c613f8d36d Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Mon, 22 Aug 2022 14:40:43 -0400 Subject: [PATCH 6/6] Update socket.ts --- packages/client/lib/client/socket.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 1bf012d7b28..2a955159323 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -5,8 +5,6 @@ import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; -type ReconnectStrategy = ; - export interface RedisSocketCommonOptions { connectTimeout?: number; noDelay?: boolean;