From 2db17827ec20fd9717950236225b55732f3a778a Mon Sep 17 00:00:00 2001 From: Jonas Faure Date: Sun, 16 Oct 2022 17:20:18 +0200 Subject: [PATCH 1/4] fix(client): timeout issues during tests --- packages/client/lib/client/socket.spec.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 1b2d050c012..5c76d5e0216 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -1,5 +1,5 @@ import { strict as assert } from 'assert'; -import { SinonFakeTimers, useFakeTimers, spy } from 'sinon'; +import { spy } from 'sinon'; import RedisSocket, { RedisSocketOptions } from './socket'; describe('Socket', () => { @@ -18,28 +18,26 @@ describe('Socket', () => { } describe('reconnectStrategy', () => { - let clock: SinonFakeTimers; - beforeEach(() => clock = useFakeTimers()); - afterEach(() => clock.restore()); - it('custom strategy', async () => { + const numberOfRetries = 10 + const reconnectStrategy = spy((retries: number) => { assert.equal(retries + 1, reconnectStrategy.callCount); - if (retries === 50) return new Error('50'); + if (retries === numberOfRetries) return new Error(`${numberOfRetries}`); const time = retries * 2; - queueMicrotask(() => clock.tick(time)); return time; }); const socket = createSocket({ host: 'error', - reconnectStrategy + reconnectStrategy, + connectTimeout:100, }); await assert.rejects(socket.connect(), { - message: '50' + message: `${numberOfRetries}` }); assert.equal(socket.isOpen, false); @@ -48,9 +46,9 @@ describe('Socket', () => { it('should handle errors', async () => { const socket = createSocket({ host: 'error', + connectTimeout:100, reconnectStrategy(retries: number) { if (retries === 1) return new Error('done'); - queueMicrotask(() => clock.tick(500)); throw new Error(); } }); From 726c15d8a65ed36f76c70d80cf7685632606cdbc Mon Sep 17 00:00:00 2001 From: Jonas Faure Date: Sun, 16 Oct 2022 17:30:09 +0200 Subject: [PATCH 2/4] fix(client): avoiding infinite Promise chaining while socket creation fails --- packages/client/lib/client/socket.ts | 63 +++++++++++++++------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 2a955159323..16b58ac538d 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -105,46 +105,49 @@ export default class RedisSocket extends EventEmitter { throw new Error('Socket already opened'); } - return this.#connect(0); + return this.#connect(); } - async #connect(retries: number, hadError?: boolean): Promise { - if (retries > 0 || hadError) { - this.emit('reconnecting'); - } - - try { - this.#isOpen = true; - this.#socket = await this.#createSocket(); - this.#writableNeedDrain = false; - this.emit('connect'); + async #connect(hadError?: boolean): Promise { + let retries = 0 + do { + if (retries > 0 || hadError) { + this.emit('reconnecting'); + } try { - await this.#initiator(); + this.#isOpen = true; + this.#socket = await this.#createSocket(); + this.#writableNeedDrain = false; + this.emit('connect'); + + try { + await this.#initiator(); + } catch (err) { + this.#socket.destroy(); + this.#socket = undefined; + throw err; + } + this.#isReady = true; + this.emit('ready'); } catch (err) { - this.#socket.destroy(); - this.#socket = undefined; - throw err; - } - this.#isReady = true; - this.emit('ready'); - } catch (err) { - const retryIn = this.reconnectStrategy(retries); - if (retryIn instanceof Error) { - this.#isOpen = false; + 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); - throw new ReconnectStrategyError(retryIn, err); + await promiseTimeout(retryIn); } - - this.emit('error', err); - await promiseTimeout(retryIn); - return this.#connect(retries + 1); - } + retries++ + } while (!this.#isReady) } #createSocket(): Promise { return new Promise((resolve, reject) => { - const {connectEvent, socket} = RedisSocket.#isTlsSocket(this.#options) ? + const { connectEvent, socket } = RedisSocket.#isTlsSocket(this.#options) ? this.#createTlsSocket() : this.#createNetSocket(); @@ -200,7 +203,7 @@ export default class RedisSocket extends EventEmitter { this.#isReady = false; this.emit('error', err); - this.#connect(0, true).catch(() => { + this.#connect(true).catch(() => { // the error was already emitted, silently ignore it }); } From 4b77aaf8e0dd3767e2a4dd20cc67a8e575402191 Mon Sep 17 00:00:00 2001 From: Jonas Faure Date: Sun, 16 Oct 2022 17:52:53 +0200 Subject: [PATCH 3/4] fix(client): Added missing semicolons --- packages/client/lib/client/socket.spec.ts | 2 +- packages/client/lib/client/socket.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 5c76d5e0216..de88b2de9e0 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -19,7 +19,7 @@ describe('Socket', () => { describe('reconnectStrategy', () => { it('custom strategy', async () => { - const numberOfRetries = 10 + const numberOfRetries = 10; const reconnectStrategy = spy((retries: number) => { assert.equal(retries + 1, reconnectStrategy.callCount); diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 16b58ac538d..fabc22038d0 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -109,7 +109,7 @@ export default class RedisSocket extends EventEmitter { } async #connect(hadError?: boolean): Promise { - let retries = 0 + let retries = 0; do { if (retries > 0 || hadError) { this.emit('reconnecting'); @@ -141,8 +141,8 @@ export default class RedisSocket extends EventEmitter { this.emit('error', err); await promiseTimeout(retryIn); } - retries++ - } while (!this.#isReady) + retries++; + } while (!this.#isReady); } #createSocket(): Promise { From 7d7cff72d3009978ad084f5c926b19d93b0d4867 Mon Sep 17 00:00:00 2001 From: leibale Date: Fri, 21 Oct 2022 10:24:30 -0400 Subject: [PATCH 4/4] clean test --- packages/client/lib/client/socket.spec.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index de88b2de9e0..c5862130cf5 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -9,9 +9,8 @@ describe('Socket', () => { options ); - socket.on('error', (err) => { + socket.on('error', () => { // ignore errors - console.log(err); }); return socket; @@ -32,8 +31,8 @@ describe('Socket', () => { const socket = createSocket({ host: 'error', - reconnectStrategy, - connectTimeout:100, + connectTimeout: 1, + reconnectStrategy }); await assert.rejects(socket.connect(), { @@ -46,7 +45,7 @@ describe('Socket', () => { it('should handle errors', async () => { const socket = createSocket({ host: 'error', - connectTimeout:100, + connectTimeout: 1, reconnectStrategy(retries: number) { if (retries === 1) return new Error('done'); throw new Error();