From ec1b7a0a9de6b5ea0c6bfda6108f897fade08dbc Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 26 Jan 2022 19:14:29 -0500 Subject: [PATCH 01/24] parser --- packages/client/lib/client/RESP/2.spec.ts | 230 +++++++++++++++ packages/client/lib/client/RESP/2.ts | 272 ++++++++++++++++++ packages/client/lib/client/RESP/3.ts | 0 .../lib/client/RESP/composers/buffer.ts | 14 + .../lib/client/RESP/composers/interface.ts | 5 + .../lib/client/RESP/composers/string.ts | 18 ++ packages/client/lib/client/RESP/example | 8 + packages/client/lib/client/RESP/interface.ts | 11 + packages/client/lib/client/RESP/test.js | 12 + packages/client/lib/client/index.ts | 8 +- packages/client/lib/errors.ts | 7 + packages/client/package.json | 2 +- 12 files changed, 583 insertions(+), 4 deletions(-) create mode 100644 packages/client/lib/client/RESP/2.spec.ts create mode 100644 packages/client/lib/client/RESP/2.ts create mode 100644 packages/client/lib/client/RESP/3.ts create mode 100644 packages/client/lib/client/RESP/composers/buffer.ts create mode 100644 packages/client/lib/client/RESP/composers/interface.ts create mode 100644 packages/client/lib/client/RESP/composers/string.ts create mode 100644 packages/client/lib/client/RESP/example create mode 100644 packages/client/lib/client/RESP/interface.ts create mode 100644 packages/client/lib/client/RESP/test.js diff --git a/packages/client/lib/client/RESP/2.spec.ts b/packages/client/lib/client/RESP/2.spec.ts new file mode 100644 index 00000000000..d5a63df5cb7 --- /dev/null +++ b/packages/client/lib/client/RESP/2.spec.ts @@ -0,0 +1,230 @@ +import { SinonSpy, spy } from 'sinon'; +import RESP2 from './2'; +import { strict as assert } from 'assert'; +import { ErrorReply } from '../../errors'; + +function generateTests( + returnStringsAsBuffers: boolean, + toWrite: Buffer, + validateReply: (replySpy: SinonSpy, errorReplySpy: SinonSpy) => void +) { + generateTest( + 'should handle single chunk', + returnStringsAsBuffers, + codec => codec.write(toWrite), + validateReply + ); + + generateTest( + 'should handle chunked string', + returnStringsAsBuffers, + codec => { + for (let i = 0; i < toWrite.length;) { + codec.write(toWrite.slice(i, ++i)); + } + }, + validateReply + ); +} + +function generateTest( + title: string, + returnStringsAsBuffers: boolean, + write: (codec: RESP2) => void, + validateReply: (replySpy: SinonSpy, errorReplySpy: SinonSpy) => void +): void { + it(title, () => { + const replySpy = spy(), + errorReplySpy = spy(), + codec = new RESP2({ + reply: replySpy, + errorReply: errorReplySpy, + returnStringsAsBuffers: () => returnStringsAsBuffers + }); + write(codec); + validateReply(replySpy, errorReplySpy); + }); +} + +describe.only('RESP 2', () => { + describe('Simple String', () => { + describe('as strings', () => { + generateTests( + false, + Buffer.from('+OK\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + ['OK'] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + + describe('as buffers', () => { + generateTests( + true, + Buffer.from('+OK\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + [Buffer.from('OK')] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + }); + + describe('Error', () => { + generateTests( + false, + Buffer.from('-ERR\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 0); + assert.equal(errorReplySpy.callCount, 1); + assert.deepEqual( + errorReplySpy.getCall(0).args, + [new ErrorReply('ERR')] + ); + } + ); + }); + + describe('Integer', () => { + generateTests( + false, + Buffer.from(':0\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + [0] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + + describe('Bulk String', () => { + describe('as strings', () => { + generateTests( + false, + Buffer.from('$6\r\nstring\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + ['string'] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + + describe('as buffers', () => { + generateTests( + true, + Buffer.from('$6\r\nstring\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + [Buffer.from('string')] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + }); + + describe('Array', () => { + describe('null', () => { + generateTests( + false, + Buffer.from('*-1\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + [null] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + + describe('empty array', () => { + generateTests( + false, + Buffer.from('*0\r\n'), + (replySpy, errorReplySpy) => { + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + [[]] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + + describe.only('as strings', () => { + generateTests( + false, + Buffer.from([ + '*7\r\n', + '+OK\r\n', + '-ERR\r\n', + ':0\r\n', + '$6\r\nstring\r\n', + '$-1\r\n', + '*-1\r\n', + '*0\r\n' + ].join('')), + (replySpy, errorReplySpy) => { + console.log(replySpy.getCalls().map(call => call.args)); + assert.equal(replySpy.callCount, 1); + assert.deepEqual( + replySpy.getCall(0).args, + [[ + 'OK', + new ErrorReply('ERR'), + 0, + 'string', + null, + null, + [] + ]] + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + + describe.skip('as buffers', () => { + generateTests( + true, + Buffer.from([ + '*7\r\n', + '+OK\r\n', + '-ERR\r\n', + ':0\r\n', + '$6\r\nstring\r\n', + '$-1\r\n', + '*-1\r\n', + '*0\r\n' + ].join('')), + (replySpy, errorReplySpy) => { + assert.ok( + replySpy.calledOnceWithExactly([]) + ); + assert.equal(errorReplySpy.callCount, 0); + } + ); + }); + }); +}); \ No newline at end of file diff --git a/packages/client/lib/client/RESP/2.ts b/packages/client/lib/client/RESP/2.ts new file mode 100644 index 00000000000..367e4adfc54 --- /dev/null +++ b/packages/client/lib/client/RESP/2.ts @@ -0,0 +1,272 @@ +import { ErrorReply } from '../../errors'; +import BufferComposer from './composers/buffer'; +import { Composer } from './composers/interface'; +import StringComposer from './composers/string'; + +enum Types { + SIMPLE_STRING = 43, // + + ERROR = 45, // - + INTEGER = 58, // : + BULK_STRING = 36, // $ + ARRAY = 42 // * +} + +const CR = 13, // \r + LF = 10; // \n + +type PrimitiveReply = string | Buffer | number | null; + +type ArrayReply = Array; + +type Reply = PrimitiveReply | ArrayReply; + +interface RESP2Options { + reply(reply?: Reply): unknown; + errorReply(errorReply?: ErrorReply): unknown; + returnStringsAsBuffers(): boolean; +} + +interface ArrayInProcess { + remaining: number; + array: ArrayReply; +} + +export default class RESP2 { + #options: RESP2Options; + + constructor(options: RESP2Options) { + this.#options = options; + } + + #type?: Types; + + #cursor = 0; + + #stringComposer = new StringComposer(); + + #bufferComposer = new BufferComposer(); + + #composer: StringComposer | BufferComposer = this.#stringComposer; + + #setComposer(): StringComposer | BufferComposer { + return this.#composer = this.#options.returnStringsAsBuffers() ? + this.#bufferComposer : + this.#stringComposer; + } + + write(buffer: Buffer): void { + while (this.#cursor < buffer.length) { + const initiate = !this.#type; + if (initiate) { + this.#type = buffer[this.#cursor++]; + } + + const reply = this.#parseType( + buffer, + this.#type!, + initiate, + true + ); + if (reply === undefined) break; + + if (reply instanceof ErrorReply) { + this.#options.errorReply(reply); + } else { + this.#options.reply(reply); + } + } + + this.#cursor -= buffer.length; + } + + #parseType( + buffer: Buffer, + type: Types, + initiate: boolean, + setComposer: boolean + ): Reply | ErrorReply | undefined { + switch (type) { + case Types.SIMPLE_STRING: + return this.#parseSimpleString( + buffer, + setComposer ? this.#setComposer() : this.#composer + ); + + case Types.ERROR: + return this.#parseError(buffer); + + case Types.INTEGER: + return this.#parseInteger(buffer); + + case Types.BULK_STRING: + if (setComposer) this.#setComposer(); + + return this.#parseBulkString(buffer); + + case Types.ARRAY: + return this.#parseArray(buffer, initiate); + } + } + + #parseSimpleString< + T extends Composer, + R = T extends Composer ? R : never + >( + buffer: Buffer, + composer: T + ): R | undefined { + if (this.#cursor >= buffer.length) return; + + const crIndex = this.#findCRLF(buffer); + if (crIndex !== -1) { + const reply = composer.end( + buffer.slice(this.#cursor, crIndex) + ); + this.#cursor = crIndex + 2; + return reply; + } + + const toWrite = buffer.slice(this.#cursor); + composer.write(toWrite); + this.#cursor = toWrite.length; + } + + #findCRLF(buffer: Buffer): number { + for (let i = this.#cursor; i < buffer.length; i++) { + if (buffer[i] === CR) { + return i; + } + } + + return -1; + } + + #parseError(buffer: Buffer): ErrorReply | undefined { + const message = this.#parseSimpleString(buffer, this.#stringComposer); + if (message !== undefined) { + return new ErrorReply(message); + } + } + + #parseInteger(buffer: Buffer): number | undefined { + const number = this.#parseSimpleString(buffer, this.#stringComposer); + if (number !== undefined) { + return Number(number); + } + } + + #bulkStringRemainingLength?: number; + + #parseBulkString(buffer: Buffer): string | Buffer | null | undefined { + if (this.#cursor >= buffer.length) return; + + if (this.#bulkStringRemainingLength === undefined) { + const remainingLength = this.#parseInteger(buffer); + if (remainingLength === undefined) return; + else if (remainingLength === -1) return null; + + this.#bulkStringRemainingLength = remainingLength; + + if (this.#cursor >= buffer.length) return; + } + + const end = this.#cursor + this.#bulkStringRemainingLength; + if (buffer.length > end) { + const reply = this.#composer.end( + buffer.slice(this.#cursor, end) + ); + this.#bulkStringRemainingLength = undefined; + this.#cursor = end + 2; + return reply; + } + + const toWrite = buffer.slice(this.#cursor); + this.#composer.write(toWrite); + this.#bulkStringRemainingLength -= toWrite.length; + this.#cursor = toWrite.length; + } + + #initiateArray = false; + + #arrays: Array = []; + + #arrayItemType?: Types; + + #parseArray(buffer: Buffer, initiate: boolean): ArrayReply | null | undefined { + let arrayInProcess: ArrayInProcess; + if (initiate || this.#initiateArray) { + const length = this.#parseInteger(buffer); + if (length === undefined) { + this.#initiateArray = true; + return; + } + + this.#initiateArray = false; + this.#arrayItemType = undefined; + + if (length === -1) { + return null; + } else if (length === 0) { + return []; + } + + this.#arrays.push(arrayInProcess = { + remaining: length, + array: new Array(length) + }); + } else { + arrayInProcess = this.#arrays[this.#arrays.length - 1]; + } + + while (this.#cursor < buffer.length) { + const initiate = !this.#arrayItemType; + if (initiate) { + this.#arrayItemType = buffer[this.#cursor++]; + } + + const item = this.#parseType( + buffer, + this.#arrayItemType!, + initiate, + false + ); + + console.log(Types[this.#arrayItemType!], item); + + if (item === undefined) break; + + this.#arrayItemType = undefined; + + const reply = this.#pushArrayReply(item); + if (reply !== undefined) return reply; + } + } + + // #returnArrayReply(reply: ArrayReply | null): ArrayReply | null | undefined { + // console.log('#returnArrayReply', this.#arrays.length, reply); + // if (!this.#arrays.length) return reply; + + // return this.#pushArrayReply(reply); + // } + + #pushArrayReply(reply: Reply | ErrorReply): ArrayReply | undefined { + let to = this.#arrays[this.#arrays.length - 1]; + to.array[to.array.length - to.remaining] = reply; + + while (--to.remaining === 0) { + if (this.#arrays.length === 0) { + return to.array; + } + + this.#arrays.pop(); + reply = to.array; + to = this.#arrays[this.#arrays.length - 1]; + } + } +} + +function log(buffer: Buffer): string { + return buffer.toString() + .replace(/\r/g, '') + .replace(/\n/g, ''); +} \ No newline at end of file diff --git a/packages/client/lib/client/RESP/3.ts b/packages/client/lib/client/RESP/3.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/client/lib/client/RESP/composers/buffer.ts b/packages/client/lib/client/RESP/composers/buffer.ts new file mode 100644 index 00000000000..9ef8a95835a --- /dev/null +++ b/packages/client/lib/client/RESP/composers/buffer.ts @@ -0,0 +1,14 @@ +import { Composer } from './interface'; + +export default class BufferComposer implements Composer { + #chunks: Array = []; + + write(buffer: Buffer): void { + this.#chunks.push(buffer); + } + + end(buffer: Buffer): Buffer { + this.write(buffer); + return Buffer.concat(this.#chunks.splice(0)); + } +} diff --git a/packages/client/lib/client/RESP/composers/interface.ts b/packages/client/lib/client/RESP/composers/interface.ts new file mode 100644 index 00000000000..6e7106c9cf9 --- /dev/null +++ b/packages/client/lib/client/RESP/composers/interface.ts @@ -0,0 +1,5 @@ +export interface Composer { + write(buffer: Buffer): void; + + end(buffer: Buffer): T; +} diff --git a/packages/client/lib/client/RESP/composers/string.ts b/packages/client/lib/client/RESP/composers/string.ts new file mode 100644 index 00000000000..646318493c2 --- /dev/null +++ b/packages/client/lib/client/RESP/composers/string.ts @@ -0,0 +1,18 @@ +import { StringDecoder } from 'string_decoder'; +import { Composer } from './interface'; + +export default class StringComposer implements Composer { + #decoder = new StringDecoder(); + + #chunks: Array = []; + + write(buffer: Buffer): void { + this.#chunks.push( + this.#decoder.write(buffer) + ); + } + + end(buffer: Buffer): string { + return this.#chunks.splice(0).join('') + this.#decoder.end(buffer); + } +} diff --git a/packages/client/lib/client/RESP/example b/packages/client/lib/client/RESP/example new file mode 100644 index 00000000000..86a49eb68c7 --- /dev/null +++ b/packages/client/lib/client/RESP/example @@ -0,0 +1,8 @@ +*7\r\n + +OK\r\n + -ERR\r\n + :0\r\n + $6\r\nstring\r\n + $-1\r\n + *-1\r\n + *0\r\n \ No newline at end of file diff --git a/packages/client/lib/client/RESP/interface.ts b/packages/client/lib/client/RESP/interface.ts new file mode 100644 index 00000000000..3deed8e373e --- /dev/null +++ b/packages/client/lib/client/RESP/interface.ts @@ -0,0 +1,11 @@ +interface RESPDecoderOptions { + returnError(err: unknown): void; + returnReply(reply: unknown): void; + returnBuffers(): boolean; +} + +export abstract class RESPDecoder { + constructor(protected options?: RESPDecoderOptions) {} + + abstract decode(buffer: Buffer): void; +} diff --git a/packages/client/lib/client/RESP/test.js b/packages/client/lib/client/RESP/test.js new file mode 100644 index 00000000000..69e757dac34 --- /dev/null +++ b/packages/client/lib/client/RESP/test.js @@ -0,0 +1,12 @@ +const parser = new (require('redis-parser'))({ + returnReply(reply) { + console.log(reply); + }, + returnError(err) { + console.error(err); + } +}); + +parser.execute( + Buffer.from('*1\r\n*1\r\n*1\r\n$-1\r\n$-1\r\n') +); diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 80f029a406e..51fbde91c18 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -35,9 +35,11 @@ type ConvertArgumentType = Type extends Set ? Set> : ( Type extends Map ? Map> : ( Type extends Array ? Array> : ( - Type extends Record ? { - [Property in keyof Type]: ConvertArgumentType - } : Type + Type extends Date ? Type : ( + Type extends Record ? { + [Property in keyof Type]: ConvertArgumentType + } : Type + ) ) ) ) diff --git a/packages/client/lib/errors.ts b/packages/client/lib/errors.ts index e43dbc81422..a1fe592d915 100644 --- a/packages/client/lib/errors.ts +++ b/packages/client/lib/errors.ts @@ -56,3 +56,10 @@ export class ReconnectStrategyError extends Error { this.socketError = socketError; } } + +export class ErrorReply extends Error { + constructor(message: string) { + super(message); + this.stack = undefined; + } +} diff --git a/packages/client/package.json b/packages/client/package.json index 036bcc858c7..7b3b5b96120 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -8,7 +8,7 @@ "dist/" ], "scripts": { - "test": "nyc -r text-summary -r lcov mocha -r source-map-support/register -r ts-node/register './lib/**/*.spec.ts'", + "test": "nyc -r text-summary -r lcov mocha -r source-map-support/register -r ts-node/register './lib/client/RESP/*.spec.ts'", "build": "tsc", "lint": "eslint ./*.ts ./lib/**/*.ts", "documentation": "typedoc" From b0b2464e9564b6e6f0e272bb12e4af97e7f11e33 Mon Sep 17 00:00:00 2001 From: leibale Date: Mon, 7 Feb 2022 15:08:15 -0500 Subject: [PATCH 02/24] a new RESP parser :) --- packages/client/lib/client/RESP/2.spec.ts | 230 --------------- packages/client/lib/client/RESP/2.ts | 272 ------------------ packages/client/lib/client/RESP/3.ts | 0 packages/client/lib/client/RESP/example | 8 - packages/client/lib/client/RESP/interface.ts | 11 - packages/client/lib/client/RESP/test.js | 12 - .../lib/client/RESP2/composers/buffer.spec.ts | 14 + .../{RESP => RESP2}/composers/buffer.ts | 0 .../{RESP => RESP2}/composers/interface.ts | 0 .../lib/client/RESP2/composers/string.spec.ts | 14 + .../{RESP => RESP2}/composers/string.ts | 0 .../client/lib/client/RESP2/decoder.spec.ts | 184 ++++++++++++ packages/client/lib/client/RESP2/decoder.ts | 233 +++++++++++++++ .../client/lib/client/RESP2/encoder.spec.ts | 33 +++ packages/client/lib/client/RESP2/encoder.ts | 34 +++ packages/client/lib/client/commands-queue.ts | 107 ++++--- packages/client/lib/client/index.ts | 16 +- packages/client/lib/client/socket.spec.ts | 12 +- packages/client/lib/client/socket.ts | 41 +-- packages/client/lib/commander.spec.ts | 36 --- packages/client/lib/commander.ts | 33 --- packages/client/lib/test-utils.ts | 2 +- packages/client/package.json | 4 +- packages/client/tsconfig.json | 3 + 24 files changed, 608 insertions(+), 691 deletions(-) delete mode 100644 packages/client/lib/client/RESP/2.spec.ts delete mode 100644 packages/client/lib/client/RESP/2.ts delete mode 100644 packages/client/lib/client/RESP/3.ts delete mode 100644 packages/client/lib/client/RESP/example delete mode 100644 packages/client/lib/client/RESP/interface.ts delete mode 100644 packages/client/lib/client/RESP/test.js create mode 100644 packages/client/lib/client/RESP2/composers/buffer.spec.ts rename packages/client/lib/client/{RESP => RESP2}/composers/buffer.ts (100%) rename packages/client/lib/client/{RESP => RESP2}/composers/interface.ts (100%) create mode 100644 packages/client/lib/client/RESP2/composers/string.spec.ts rename packages/client/lib/client/{RESP => RESP2}/composers/string.ts (100%) create mode 100644 packages/client/lib/client/RESP2/decoder.spec.ts create mode 100644 packages/client/lib/client/RESP2/decoder.ts create mode 100644 packages/client/lib/client/RESP2/encoder.spec.ts create mode 100644 packages/client/lib/client/RESP2/encoder.ts delete mode 100644 packages/client/lib/commander.spec.ts diff --git a/packages/client/lib/client/RESP/2.spec.ts b/packages/client/lib/client/RESP/2.spec.ts deleted file mode 100644 index d5a63df5cb7..00000000000 --- a/packages/client/lib/client/RESP/2.spec.ts +++ /dev/null @@ -1,230 +0,0 @@ -import { SinonSpy, spy } from 'sinon'; -import RESP2 from './2'; -import { strict as assert } from 'assert'; -import { ErrorReply } from '../../errors'; - -function generateTests( - returnStringsAsBuffers: boolean, - toWrite: Buffer, - validateReply: (replySpy: SinonSpy, errorReplySpy: SinonSpy) => void -) { - generateTest( - 'should handle single chunk', - returnStringsAsBuffers, - codec => codec.write(toWrite), - validateReply - ); - - generateTest( - 'should handle chunked string', - returnStringsAsBuffers, - codec => { - for (let i = 0; i < toWrite.length;) { - codec.write(toWrite.slice(i, ++i)); - } - }, - validateReply - ); -} - -function generateTest( - title: string, - returnStringsAsBuffers: boolean, - write: (codec: RESP2) => void, - validateReply: (replySpy: SinonSpy, errorReplySpy: SinonSpy) => void -): void { - it(title, () => { - const replySpy = spy(), - errorReplySpy = spy(), - codec = new RESP2({ - reply: replySpy, - errorReply: errorReplySpy, - returnStringsAsBuffers: () => returnStringsAsBuffers - }); - write(codec); - validateReply(replySpy, errorReplySpy); - }); -} - -describe.only('RESP 2', () => { - describe('Simple String', () => { - describe('as strings', () => { - generateTests( - false, - Buffer.from('+OK\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - ['OK'] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - - describe('as buffers', () => { - generateTests( - true, - Buffer.from('+OK\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - [Buffer.from('OK')] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - }); - - describe('Error', () => { - generateTests( - false, - Buffer.from('-ERR\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 0); - assert.equal(errorReplySpy.callCount, 1); - assert.deepEqual( - errorReplySpy.getCall(0).args, - [new ErrorReply('ERR')] - ); - } - ); - }); - - describe('Integer', () => { - generateTests( - false, - Buffer.from(':0\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - [0] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - - describe('Bulk String', () => { - describe('as strings', () => { - generateTests( - false, - Buffer.from('$6\r\nstring\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - ['string'] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - - describe('as buffers', () => { - generateTests( - true, - Buffer.from('$6\r\nstring\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - [Buffer.from('string')] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - }); - - describe('Array', () => { - describe('null', () => { - generateTests( - false, - Buffer.from('*-1\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - [null] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - - describe('empty array', () => { - generateTests( - false, - Buffer.from('*0\r\n'), - (replySpy, errorReplySpy) => { - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - [[]] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - - describe.only('as strings', () => { - generateTests( - false, - Buffer.from([ - '*7\r\n', - '+OK\r\n', - '-ERR\r\n', - ':0\r\n', - '$6\r\nstring\r\n', - '$-1\r\n', - '*-1\r\n', - '*0\r\n' - ].join('')), - (replySpy, errorReplySpy) => { - console.log(replySpy.getCalls().map(call => call.args)); - assert.equal(replySpy.callCount, 1); - assert.deepEqual( - replySpy.getCall(0).args, - [[ - 'OK', - new ErrorReply('ERR'), - 0, - 'string', - null, - null, - [] - ]] - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - - describe.skip('as buffers', () => { - generateTests( - true, - Buffer.from([ - '*7\r\n', - '+OK\r\n', - '-ERR\r\n', - ':0\r\n', - '$6\r\nstring\r\n', - '$-1\r\n', - '*-1\r\n', - '*0\r\n' - ].join('')), - (replySpy, errorReplySpy) => { - assert.ok( - replySpy.calledOnceWithExactly([]) - ); - assert.equal(errorReplySpy.callCount, 0); - } - ); - }); - }); -}); \ No newline at end of file diff --git a/packages/client/lib/client/RESP/2.ts b/packages/client/lib/client/RESP/2.ts deleted file mode 100644 index 367e4adfc54..00000000000 --- a/packages/client/lib/client/RESP/2.ts +++ /dev/null @@ -1,272 +0,0 @@ -import { ErrorReply } from '../../errors'; -import BufferComposer from './composers/buffer'; -import { Composer } from './composers/interface'; -import StringComposer from './composers/string'; - -enum Types { - SIMPLE_STRING = 43, // + - ERROR = 45, // - - INTEGER = 58, // : - BULK_STRING = 36, // $ - ARRAY = 42 // * -} - -const CR = 13, // \r - LF = 10; // \n - -type PrimitiveReply = string | Buffer | number | null; - -type ArrayReply = Array; - -type Reply = PrimitiveReply | ArrayReply; - -interface RESP2Options { - reply(reply?: Reply): unknown; - errorReply(errorReply?: ErrorReply): unknown; - returnStringsAsBuffers(): boolean; -} - -interface ArrayInProcess { - remaining: number; - array: ArrayReply; -} - -export default class RESP2 { - #options: RESP2Options; - - constructor(options: RESP2Options) { - this.#options = options; - } - - #type?: Types; - - #cursor = 0; - - #stringComposer = new StringComposer(); - - #bufferComposer = new BufferComposer(); - - #composer: StringComposer | BufferComposer = this.#stringComposer; - - #setComposer(): StringComposer | BufferComposer { - return this.#composer = this.#options.returnStringsAsBuffers() ? - this.#bufferComposer : - this.#stringComposer; - } - - write(buffer: Buffer): void { - while (this.#cursor < buffer.length) { - const initiate = !this.#type; - if (initiate) { - this.#type = buffer[this.#cursor++]; - } - - const reply = this.#parseType( - buffer, - this.#type!, - initiate, - true - ); - if (reply === undefined) break; - - if (reply instanceof ErrorReply) { - this.#options.errorReply(reply); - } else { - this.#options.reply(reply); - } - } - - this.#cursor -= buffer.length; - } - - #parseType( - buffer: Buffer, - type: Types, - initiate: boolean, - setComposer: boolean - ): Reply | ErrorReply | undefined { - switch (type) { - case Types.SIMPLE_STRING: - return this.#parseSimpleString( - buffer, - setComposer ? this.#setComposer() : this.#composer - ); - - case Types.ERROR: - return this.#parseError(buffer); - - case Types.INTEGER: - return this.#parseInteger(buffer); - - case Types.BULK_STRING: - if (setComposer) this.#setComposer(); - - return this.#parseBulkString(buffer); - - case Types.ARRAY: - return this.#parseArray(buffer, initiate); - } - } - - #parseSimpleString< - T extends Composer, - R = T extends Composer ? R : never - >( - buffer: Buffer, - composer: T - ): R | undefined { - if (this.#cursor >= buffer.length) return; - - const crIndex = this.#findCRLF(buffer); - if (crIndex !== -1) { - const reply = composer.end( - buffer.slice(this.#cursor, crIndex) - ); - this.#cursor = crIndex + 2; - return reply; - } - - const toWrite = buffer.slice(this.#cursor); - composer.write(toWrite); - this.#cursor = toWrite.length; - } - - #findCRLF(buffer: Buffer): number { - for (let i = this.#cursor; i < buffer.length; i++) { - if (buffer[i] === CR) { - return i; - } - } - - return -1; - } - - #parseError(buffer: Buffer): ErrorReply | undefined { - const message = this.#parseSimpleString(buffer, this.#stringComposer); - if (message !== undefined) { - return new ErrorReply(message); - } - } - - #parseInteger(buffer: Buffer): number | undefined { - const number = this.#parseSimpleString(buffer, this.#stringComposer); - if (number !== undefined) { - return Number(number); - } - } - - #bulkStringRemainingLength?: number; - - #parseBulkString(buffer: Buffer): string | Buffer | null | undefined { - if (this.#cursor >= buffer.length) return; - - if (this.#bulkStringRemainingLength === undefined) { - const remainingLength = this.#parseInteger(buffer); - if (remainingLength === undefined) return; - else if (remainingLength === -1) return null; - - this.#bulkStringRemainingLength = remainingLength; - - if (this.#cursor >= buffer.length) return; - } - - const end = this.#cursor + this.#bulkStringRemainingLength; - if (buffer.length > end) { - const reply = this.#composer.end( - buffer.slice(this.#cursor, end) - ); - this.#bulkStringRemainingLength = undefined; - this.#cursor = end + 2; - return reply; - } - - const toWrite = buffer.slice(this.#cursor); - this.#composer.write(toWrite); - this.#bulkStringRemainingLength -= toWrite.length; - this.#cursor = toWrite.length; - } - - #initiateArray = false; - - #arrays: Array = []; - - #arrayItemType?: Types; - - #parseArray(buffer: Buffer, initiate: boolean): ArrayReply | null | undefined { - let arrayInProcess: ArrayInProcess; - if (initiate || this.#initiateArray) { - const length = this.#parseInteger(buffer); - if (length === undefined) { - this.#initiateArray = true; - return; - } - - this.#initiateArray = false; - this.#arrayItemType = undefined; - - if (length === -1) { - return null; - } else if (length === 0) { - return []; - } - - this.#arrays.push(arrayInProcess = { - remaining: length, - array: new Array(length) - }); - } else { - arrayInProcess = this.#arrays[this.#arrays.length - 1]; - } - - while (this.#cursor < buffer.length) { - const initiate = !this.#arrayItemType; - if (initiate) { - this.#arrayItemType = buffer[this.#cursor++]; - } - - const item = this.#parseType( - buffer, - this.#arrayItemType!, - initiate, - false - ); - - console.log(Types[this.#arrayItemType!], item); - - if (item === undefined) break; - - this.#arrayItemType = undefined; - - const reply = this.#pushArrayReply(item); - if (reply !== undefined) return reply; - } - } - - // #returnArrayReply(reply: ArrayReply | null): ArrayReply | null | undefined { - // console.log('#returnArrayReply', this.#arrays.length, reply); - // if (!this.#arrays.length) return reply; - - // return this.#pushArrayReply(reply); - // } - - #pushArrayReply(reply: Reply | ErrorReply): ArrayReply | undefined { - let to = this.#arrays[this.#arrays.length - 1]; - to.array[to.array.length - to.remaining] = reply; - - while (--to.remaining === 0) { - if (this.#arrays.length === 0) { - return to.array; - } - - this.#arrays.pop(); - reply = to.array; - to = this.#arrays[this.#arrays.length - 1]; - } - } -} - -function log(buffer: Buffer): string { - return buffer.toString() - .replace(/\r/g, '') - .replace(/\n/g, ''); -} \ No newline at end of file diff --git a/packages/client/lib/client/RESP/3.ts b/packages/client/lib/client/RESP/3.ts deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/packages/client/lib/client/RESP/example b/packages/client/lib/client/RESP/example deleted file mode 100644 index 86a49eb68c7..00000000000 --- a/packages/client/lib/client/RESP/example +++ /dev/null @@ -1,8 +0,0 @@ -*7\r\n - +OK\r\n - -ERR\r\n - :0\r\n - $6\r\nstring\r\n - $-1\r\n - *-1\r\n - *0\r\n \ No newline at end of file diff --git a/packages/client/lib/client/RESP/interface.ts b/packages/client/lib/client/RESP/interface.ts deleted file mode 100644 index 3deed8e373e..00000000000 --- a/packages/client/lib/client/RESP/interface.ts +++ /dev/null @@ -1,11 +0,0 @@ -interface RESPDecoderOptions { - returnError(err: unknown): void; - returnReply(reply: unknown): void; - returnBuffers(): boolean; -} - -export abstract class RESPDecoder { - constructor(protected options?: RESPDecoderOptions) {} - - abstract decode(buffer: Buffer): void; -} diff --git a/packages/client/lib/client/RESP/test.js b/packages/client/lib/client/RESP/test.js deleted file mode 100644 index 69e757dac34..00000000000 --- a/packages/client/lib/client/RESP/test.js +++ /dev/null @@ -1,12 +0,0 @@ -const parser = new (require('redis-parser'))({ - returnReply(reply) { - console.log(reply); - }, - returnError(err) { - console.error(err); - } -}); - -parser.execute( - Buffer.from('*1\r\n*1\r\n*1\r\n$-1\r\n$-1\r\n') -); diff --git a/packages/client/lib/client/RESP2/composers/buffer.spec.ts b/packages/client/lib/client/RESP2/composers/buffer.spec.ts new file mode 100644 index 00000000000..f57c369fecb --- /dev/null +++ b/packages/client/lib/client/RESP2/composers/buffer.spec.ts @@ -0,0 +1,14 @@ +import { strict as assert } from 'assert'; +import BufferComposer from './buffer'; + +describe('Buffer Composer', () => { + const composer = new BufferComposer(); + + it('should compose two buffers', () => { + composer.write(Buffer.from([0])); + assert.deepEqual( + composer.end(Buffer.from([1])), + Buffer.from([0, 1]) + ); + }); +}); diff --git a/packages/client/lib/client/RESP/composers/buffer.ts b/packages/client/lib/client/RESP2/composers/buffer.ts similarity index 100% rename from packages/client/lib/client/RESP/composers/buffer.ts rename to packages/client/lib/client/RESP2/composers/buffer.ts diff --git a/packages/client/lib/client/RESP/composers/interface.ts b/packages/client/lib/client/RESP2/composers/interface.ts similarity index 100% rename from packages/client/lib/client/RESP/composers/interface.ts rename to packages/client/lib/client/RESP2/composers/interface.ts diff --git a/packages/client/lib/client/RESP2/composers/string.spec.ts b/packages/client/lib/client/RESP2/composers/string.spec.ts new file mode 100644 index 00000000000..9dd26aae021 --- /dev/null +++ b/packages/client/lib/client/RESP2/composers/string.spec.ts @@ -0,0 +1,14 @@ +import { strict as assert } from 'assert'; +import StringComposer from './string'; + +describe('String Composer', () => { + const composer = new StringComposer(); + + it('should compose two strings', () => { + composer.write(Buffer.from([0])); + assert.deepEqual( + composer.end(Buffer.from([1])), + Buffer.from([0, 1]).toString() + ); + }); +}); diff --git a/packages/client/lib/client/RESP/composers/string.ts b/packages/client/lib/client/RESP2/composers/string.ts similarity index 100% rename from packages/client/lib/client/RESP/composers/string.ts rename to packages/client/lib/client/RESP2/composers/string.ts diff --git a/packages/client/lib/client/RESP2/decoder.spec.ts b/packages/client/lib/client/RESP2/decoder.spec.ts new file mode 100644 index 00000000000..aae42fa7deb --- /dev/null +++ b/packages/client/lib/client/RESP2/decoder.spec.ts @@ -0,0 +1,184 @@ +import { strict as assert } from 'assert'; +import { SinonSpy, spy } from 'sinon'; +import RESP2Decoder from './decoder'; +import { ErrorReply } from '../../errors'; + +interface Stream { + stream: RESP2Decoder; + returnStringsAsBuffersSpy: SinonSpy; + repliesSpy: SinonSpy; +} + +function createStreamAndSpies(returnStringsAsBuffers: boolean): Stream { + const returnStringsAsBuffersSpy = spy(() => returnStringsAsBuffers), + repliesSpy = spy(); + + return { + stream: new RESP2Decoder({ returnStringsAsBuffers: returnStringsAsBuffersSpy }) + .on('data', repliesSpy), + returnStringsAsBuffersSpy, + repliesSpy + }; +} + +function writeChunks(stream: RESP2Decoder, buffer: Buffer) { + let i = 0; + while (i < buffer.length) { + stream.write(buffer.slice(i, ++i)); + } +} + +type Replies = Array>; + +interface TestsOptions { + toWrite: Buffer; + returnStringsAsBuffers: boolean; + replies: Replies; +} + +function generateTests({ + toWrite, + returnStringsAsBuffers, + replies +}: TestsOptions): void { + + it('single chunk', () => { + const { stream, returnStringsAsBuffersSpy, repliesSpy } = + createStreamAndSpies(returnStringsAsBuffers); + stream.write(toWrite); + assert.equal(returnStringsAsBuffersSpy.callCount, replies.length); + testReplies(repliesSpy, replies); + }); + + it('multiple chunks', () => { + const { stream, returnStringsAsBuffersSpy, repliesSpy } = + createStreamAndSpies(returnStringsAsBuffers); + writeChunks(stream, toWrite); + assert.equal(returnStringsAsBuffersSpy.callCount, replies.length); + testReplies(repliesSpy, replies); + }); +} + +function testReplies(spy: SinonSpy, replies: Replies): void { + if (!replies) { + assert.equal(spy.callCount, 0); + return; + } + + assert.equal(spy.callCount, replies.length); + for (const [i, reply] of replies.entries()) { + assert.deepEqual( + spy.getCall(i).args, + reply + ); + } +} + +describe('RESP2Parser', () => { + describe('Simple String', () => { + describe('as strings', () => { + generateTests({ + toWrite: Buffer.from('+OK\r\n'), + returnStringsAsBuffers: false, + replies: [['OK']] + }); + }); + + describe('as buffers', () => { + generateTests({ + toWrite: Buffer.from('+OK\r\n'), + returnStringsAsBuffers: true, + replies: [[Buffer.from('OK')]] + }); + }); + }); + + describe('Error', () => { + generateTests({ + toWrite: Buffer.from('-ERR\r\n'), + returnStringsAsBuffers: true, + replies: [[new ErrorReply('ERR')]] + }); + }); + + describe('Integer', () => { + generateTests({ + toWrite: Buffer.from(':0\r\n'), + returnStringsAsBuffers: true, + replies: [[0]] + }); + }); + + describe('Bulk String', () => { + describe('null', () => { + generateTests({ + toWrite: Buffer.from('$-1\r\n'), + returnStringsAsBuffers: false, + replies: [[null]] + }); + }); + + describe('as strings', () => { + generateTests({ + toWrite: Buffer.from('$2\r\naa\r\n'), + returnStringsAsBuffers: false, + replies: [['aa']] + }); + }); + + describe('as buffers', () => { + generateTests({ + toWrite: Buffer.from('$2\r\naa\r\n'), + returnStringsAsBuffers: true, + replies: [[Buffer.from('aa')]] + }); + }); + }); + + describe('Array', () => { + describe('null', () => { + generateTests({ + toWrite: Buffer.from('*-1\r\n'), + returnStringsAsBuffers: false, + replies: [[null]] + }); + }); + + const arrayBuffer = Buffer.from( + '*5\r\n' + + '+OK\r\n' + + '-ERR\r\n' + + ':0\r\n' + + '$1\r\na\r\n' + + '*0\r\n' + ); + + describe('as strings', () => { + generateTests({ + toWrite: arrayBuffer, + returnStringsAsBuffers: false, + replies: [[[ + 'OK', + new ErrorReply('ERR'), + 0, + 'a', + [] + ]]] + }); + }); + + describe('as buffers', () => { + generateTests({ + toWrite: arrayBuffer, + returnStringsAsBuffers: true, + replies: [[[ + Buffer.from('OK'), + new ErrorReply('ERR'), + 0, + Buffer.from('a'), + [] + ]]] + }); + }); + }); +}); diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts new file mode 100644 index 00000000000..546ddaa3c0a --- /dev/null +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -0,0 +1,233 @@ +import { Transform, TransformCallback } from 'stream'; +import { ErrorReply } from '../../errors'; +import { Composer } from './composers/interface'; +import BufferComposer from './composers/buffer'; +import StringComposer from './composers/string'; + +enum Types { + SIMPLE_STRING = 43, // + + ERROR = 45, // - + INTEGER = 58, // : + BULK_STRING = 36, // $ + ARRAY = 42 // * +} + +const CR = 13; // \r + +export type Reply = string | Buffer | ErrorReply | number | null | Array; + +type ArrayReply = Array | null; + +export type ReturnStringsAsBuffers = () => boolean; + +interface RESP2Options { + returnStringsAsBuffers: ReturnStringsAsBuffers +} + +interface ArrayInProcess { + array: Array; + pushCounter: number; +} + +export default class RESP2Decoder extends Transform { + #options: RESP2Options; + + constructor(options: RESP2Options) { + super({ readableObjectMode: true }); + + this.#options = options; + } + + #cursor = 0; + + #type?: Types; + + #bufferComposer = new BufferComposer(); + + #stringComposer = new StringComposer(); + + #composer: BufferComposer | StringComposer = this.#stringComposer; + + _transform(chunk: Buffer, _: BufferEncoding, callback: TransformCallback): void { + while (this.#cursor < chunk.length) { + if (!this.#type) { + this.#composer = this.#options.returnStringsAsBuffers() ? + this.#bufferComposer : + this.#stringComposer; + + this.#type = chunk[this.#cursor]; + if (++this.#cursor >= chunk.length) break; + } + + const reply = this.#parseType(chunk, this.#type); + if (reply === undefined) break; + + this.#type = undefined; + this.emit('data', reply); + } + + this.#cursor -= chunk.length; + callback(); + } + + #parseType(chunk: Buffer, type: Types, arraysToKeep?: number): Reply | undefined { + switch (type) { + case Types.SIMPLE_STRING: + return this.#parseSimpleString(chunk, this.#composer); + + case Types.ERROR: + return this.#parseError(chunk); + + case Types.INTEGER: + return this.#parseInteger(chunk); + + case Types.BULK_STRING: + return this.#parseBulkString(chunk); + + case Types.ARRAY: + return this.#parseArray(chunk, arraysToKeep); + } + } + + #parseSimpleString< + C extends Composer, + T = C extends Composer ? TT : never + >( + chunk: Buffer, + composer: C + ): T | undefined { + const crIndex = this.#findCRLF(chunk); + if (crIndex !== -1) { + const reply = composer.end( + chunk.slice(this.#cursor, crIndex) + ); + this.#cursor = crIndex + 2; + return reply; + } + + const toWrite = chunk.slice(this.#cursor); + composer.write(toWrite); + this.#cursor = toWrite.length; + } + + #findCRLF(chunk: Buffer): number { + for (let i = this.#cursor; i < chunk.length; i++) { + if (chunk[i] === CR) { + return i; + } + } + + return -1; + } + + #parseError(chunk: Buffer): ErrorReply | undefined { + const message = this.#parseSimpleString(chunk, this.#stringComposer); + if (message !== undefined) { + return new ErrorReply(message); + } + } + + #parseInteger(chunk: Buffer): number | undefined { + const number = this.#parseSimpleString(chunk, this.#stringComposer); + if (number !== undefined) { + return Number(number); + } + } + + #bulkStringRemainingLength?: number; + + #parseBulkString(chunk: Buffer): string | Buffer | null | undefined { + if (this.#bulkStringRemainingLength === undefined) { + const length = this.#parseInteger(chunk); + if (length === undefined) return; + else if (length === -1) return null; + + this.#bulkStringRemainingLength = length; + + + if (this.#cursor >= chunk.length) return; + } + + const end = this.#cursor + this.#bulkStringRemainingLength; + if (chunk.length >= end) { + const reply = this.#composer.end( + chunk.slice(this.#cursor, end) + ); + this.#bulkStringRemainingLength = undefined; + this.#cursor = end + 2; + return reply; + } + + const toWrite = chunk.slice(this.#cursor); + this.#composer.write(toWrite); + this.#bulkStringRemainingLength -= toWrite.length; + this.#cursor = toWrite.length; + } + + #arraysInProcess: Array = []; + + #initializeArray = false; + + #arrayItemType?: Types; + + #parseArray(chunk: Buffer, arraysToKeep = 0): ArrayReply | undefined { + if (this.#initializeArray || this.#arraysInProcess.length === arraysToKeep) { + const length = this.#parseInteger(chunk); + if (length === undefined) { + this.#initializeArray = true; + return undefined; + } + + this.#initializeArray = false; + this.#arrayItemType = undefined; + + if (length === -1) { + return this.#returnArrayReply(null, arraysToKeep); + } else if (length === 0) { + return this.#returnArrayReply([], arraysToKeep); + } + + this.#arraysInProcess.push({ + array: new Array(length), + pushCounter: 0 + }); + } + + while (this.#cursor < chunk.length) { + if (!this.#arrayItemType) { + this.#arrayItemType = chunk[this.#cursor]; + + if (++this.#cursor >= chunk.length) break; + } + + const item = this.#parseType( + chunk, + this.#arrayItemType, + arraysToKeep + 1 + ); + if (item === undefined) break; + + this.#arrayItemType = undefined; + + const reply = this.#pushArrayItem(item, arraysToKeep); + if (reply !== undefined) return reply; + } + } + + #returnArrayReply(reply: ArrayReply, arraysToKeep: number): ArrayReply | undefined { + if (this.#arraysInProcess.length <= arraysToKeep) return reply; + + return this.#pushArrayItem(reply, arraysToKeep); + } + + #pushArrayItem(item: Reply, arraysToKeep: number): ArrayReply | undefined { + const to = this.#arraysInProcess[this.#arraysInProcess.length - 1]!; + to.array[to.pushCounter] = item; + if (++to.pushCounter === to.array.length) { + return this.#returnArrayReply( + this.#arraysInProcess.pop()!.array, + arraysToKeep + ); + } + } +} diff --git a/packages/client/lib/client/RESP2/encoder.spec.ts b/packages/client/lib/client/RESP2/encoder.spec.ts new file mode 100644 index 00000000000..c1d0002391f --- /dev/null +++ b/packages/client/lib/client/RESP2/encoder.spec.ts @@ -0,0 +1,33 @@ +import { strict as assert } from 'assert'; +import { describe } from 'mocha'; +import encodeCommand from './encoder'; + +describe('RESP2 Encoder', () => { + it('1 byte', () => { + assert.deepEqual( + [...encodeCommand(['a', 'z'])], + ['*2\r\n$1\r\na\r\n$1\r\nz\r\n'] + ); + }); + + it('2 bytes', () => { + assert.deepEqual( + [...encodeCommand(['讗', '转'])], + ['*2\r\n$2\r\n讗\r\n$2\r\n转\r\n'] + ); + }); + + it('4 bytes', () => { + assert.deepEqual( + [...encodeCommand(['馃悾', '馃悿'])], + ['*2\r\n$4\r\n馃悾\r\n$4\r\n馃悿\r\n'] + ); + }); + + it('buffer', () => { + assert.deepEqual( + [...encodeCommand([Buffer.from('string')])], + ['*1\r\n$6\r\n', Buffer.from('string'), '\r\n'] + ); + }); +}); diff --git a/packages/client/lib/client/RESP2/encoder.ts b/packages/client/lib/client/RESP2/encoder.ts new file mode 100644 index 00000000000..489f14a0ecc --- /dev/null +++ b/packages/client/lib/client/RESP2/encoder.ts @@ -0,0 +1,34 @@ +import { RedisCommandArgument, RedisCommandArguments } from '../../commands'; + +const CRLF = '\r\n'; + +export default function* encodeCommand(args: RedisCommandArguments): IterableIterator { + let strings = `*${args.length}${CRLF}`, + stringsLength = 0; + for (const arg of args) { + if (Buffer.isBuffer(arg)) { + yield `${strings}$${arg.length}${CRLF}`; + strings = ''; + stringsLength = 0; + yield arg; + } else { + const string = arg?.toString?.() ?? '', + byteLength = Buffer.byteLength(string); + strings += `$${byteLength}${CRLF}`; + + const totalLength = stringsLength + byteLength; + if (totalLength > 1024) { + yield strings; + strings = string; + stringsLength = byteLength; + } else { + strings += string; + stringsLength = totalLength; + } + } + + strings += CRLF; + } + + yield strings; +} diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index df2c25dbc2c..8b3e5340afd 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -1,11 +1,7 @@ import * as LinkedList from 'yallist'; -import { AbortError } from '../errors'; +import { AbortError, ErrorReply } from '../errors'; import { RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply } from '../commands'; - -// We need to use 'require', because it's not possible with Typescript to import -// classes that are exported as 'module.exports = class`, without esModuleInterop -// set to true. -const RedisParser = require('redis-parser'); +import { Reply } from './RESP2/decoder'; export interface QueueCommandOptions { asap?: boolean; @@ -95,7 +91,6 @@ export default class RedisCommandsQueue { readonly #maxLength: number | null | undefined; readonly #waitingToBeSent = new LinkedList(); - readonly #waitingForReply = new LinkedList(); #pubSubState: PubSubState | undefined; @@ -105,44 +100,10 @@ export default class RedisCommandsQueue { pMessage: Buffer.from('pmessage'), subscribe: Buffer.from('subscribe'), pSubscribe: Buffer.from('psubscribe'), - unsubscribe: Buffer.from('unsunscribe'), + unsubscribe: Buffer.from('unsubscribe'), pUnsubscribe: Buffer.from('punsubscribe') }; - readonly #parser = new RedisParser({ - returnReply: (reply: unknown) => { - if (this.#pubSubState && Array.isArray(reply)) { - if (RedisCommandsQueue.#PUB_SUB_MESSAGES.message.equals(reply[0])) { - return RedisCommandsQueue.#emitPubSubMessage( - this.#pubSubState.listeners.channels, - reply[2], - reply[1] - ); - } else if (RedisCommandsQueue.#PUB_SUB_MESSAGES.pMessage.equals(reply[0])) { - return RedisCommandsQueue.#emitPubSubMessage( - this.#pubSubState.listeners.patterns, - reply[3], - reply[2], - reply[1] - ); - } else if ( - RedisCommandsQueue.#PUB_SUB_MESSAGES.subscribe.equals(reply[0]) || - RedisCommandsQueue.#PUB_SUB_MESSAGES.pSubscribe.equals(reply[0]) || - RedisCommandsQueue.#PUB_SUB_MESSAGES.unsubscribe.equals(reply[0]) || - RedisCommandsQueue.#PUB_SUB_MESSAGES.pUnsubscribe.equals(reply[0]) - ) { - if (--this.#waitingForReply.head!.value.channelsCounter! === 0) { - this.#shiftWaitingForReply().resolve(); - } - return; - } - } - - this.#shiftWaitingForReply().resolve(reply); - }, - returnError: (err: Error) => this.#shiftWaitingForReply().reject(err) - }); - #chainInExecution: symbol | undefined; constructor(maxLength: number | null | undefined) { @@ -272,9 +233,11 @@ export default class RedisCommandsQueue { listeners.delete(channel); } } + if (!channelsToUnsubscribe.length) { return Promise.resolve(); } + return this.#pushPubSubCommand(command, channelsToUnsubscribe); } @@ -356,36 +319,66 @@ export default class RedisCommandsQueue { const toSend = this.#waitingToBeSent.shift(); if (toSend) { this.#waitingForReply.push({ + args: toSend.args, resolve: toSend.resolve, reject: toSend.reject, channelsCounter: toSend.channelsCounter, returnBuffers: toSend.returnBuffers - }); + } as any); } this.#chainInExecution = toSend?.chainId; return toSend?.args; } - #setReturnBuffers() { - this.#parser.setReturnBuffers( - !!this.#waitingForReply.head?.value.returnBuffers || - !!this.#pubSubState?.subscribed - ); + returnStringsAsBuffers(): boolean { + return !!this.#waitingForReply.head?.value.returnBuffers || + !!this.#pubSubState; } - parseResponse(data: Buffer): void { - this.#setReturnBuffers(); - this.#parser.execute(data); + handleReply(reply: Reply): void { + if (this.#handlePubSubReply(reply)) { + return; + } else if (!this.#waitingForReply.length) { + throw new Error('Got an unexpected reply from Redis'); + } + + const { resolve, reject } = this.#waitingForReply.shift()!; + if (reply instanceof ErrorReply) { + reject(reply); + } else { + resolve(reply); + } } - #shiftWaitingForReply(): CommandWaitingForReply { - if (!this.#waitingForReply.length) { - throw new Error('Got an unexpected reply from Redis'); + #handlePubSubReply(reply: any): boolean { + if (!this.#pubSubState || !Array.isArray(reply)) return false; + + if (RedisCommandsQueue.#PUB_SUB_MESSAGES.message.equals(reply[0])) { + RedisCommandsQueue.#emitPubSubMessage( + this.#pubSubState.listeners.channels, + reply[2], + reply[1] + ); + } else if (RedisCommandsQueue.#PUB_SUB_MESSAGES.pMessage.equals(reply[0])) { + RedisCommandsQueue.#emitPubSubMessage( + this.#pubSubState.listeners.patterns, + reply[3], + reply[2], + reply[1] + ); + } else if ( + RedisCommandsQueue.#PUB_SUB_MESSAGES.subscribe.equals(reply[0]) || + RedisCommandsQueue.#PUB_SUB_MESSAGES.pSubscribe.equals(reply[0]) || + RedisCommandsQueue.#PUB_SUB_MESSAGES.unsubscribe.equals(reply[0]) || + RedisCommandsQueue.#PUB_SUB_MESSAGES.pUnsubscribe.equals(reply[0]) + ) { + const a = this.#waitingForReply.head!.value as any; + if (--this.#waitingForReply.head!.value.channelsCounter! === 0) { + this.#waitingForReply.shift()!.resolve(); + } } - const waitingForReply = this.#waitingForReply.shift()!; - this.#setReturnBuffers(); - return waitingForReply; + return true; } flushWaitingForReply(err: Error): void { diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 2ce93773e97..326a96bdf28 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -157,8 +157,8 @@ export default class RedisClient } readonly #options?: RedisClientOptions; - readonly #socket: RedisSocket; readonly #queue: RedisCommandsQueue; + readonly #socket: RedisSocket; readonly #isolationPool: Pool>; readonly #v4: Record = {}; #selectedDB = 0; @@ -182,8 +182,8 @@ export default class RedisClient constructor(options?: RedisClientOptions) { super(); this.#options = this.#initiateOptions(options); - this.#socket = this.#initiateSocket(); this.#queue = this.#initiateQueue(); + this.#socket = this.#initiateSocket(); this.#isolationPool = createPool({ create: async () => { const duplicate = this.duplicate({ @@ -214,6 +214,10 @@ export default class RedisClient return options; } + #initiateQueue(): RedisCommandsQueue { + return new RedisCommandsQueue(this.#options?.commandsQueueMaxLength); + } + #initiateSocket(): RedisSocket { const socketInitiator = async (): Promise => { const promises = []; @@ -270,8 +274,8 @@ export default class RedisClient } }; - return new RedisSocket(socketInitiator, this.#options?.socket) - .on('data', data => this.#queue.parseResponse(data)) + return new RedisSocket(socketInitiator, () => this.#queue.returnStringsAsBuffers(), this.#options?.socket) + .on('reply', reply => this.#queue.handleReply(reply)) .on('error', err => { this.emit('error', err); if (this.#socket.isOpen) { @@ -290,10 +294,6 @@ export default class RedisClient .on('end', () => this.emit('end')); } - #initiateQueue(): RedisCommandsQueue { - return new RedisCommandsQueue(this.#options?.commandsQueueMaxLength); - } - #legacyMode(): void { if (!this.#options?.legacyMode) return; diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 263320bbf72..2f30c461dc8 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -21,10 +21,14 @@ describe('Socket', () => { return time; }); - const socket = new RedisSocket(undefined, { - host: 'error', - reconnectStrategy - }); + const socket = new RedisSocket( + () => Promise.resolve(), + () => false, + { + host: 'error', + reconnectStrategy + } + ); socket.on('error', () => { // ignore errors diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 269c52381f3..2de4407e114 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -1,11 +1,11 @@ import { EventEmitter } from 'events'; import * as net from 'net'; import * as tls from 'tls'; -import { encodeCommand } from '../commander'; import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, AuthError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; - +import encodeCommand from './RESP2/encoder'; +import RESP2Decoder, { Reply, ReturnStringsAsBuffers } from './RESP2/decoder'; export interface RedisSocketCommonOptions { connectTimeout?: number; noDelay?: boolean; @@ -53,7 +53,9 @@ export default class RedisSocket extends EventEmitter { return (options as RedisTlsSocketOptions).tls === true; } - readonly #initiator?: RedisSocketInitiator; + readonly #initiator: RedisSocketInitiator; + + readonly #returnStringsAsBuffers: ReturnStringsAsBuffers; readonly #options: RedisSocketOptions; @@ -79,10 +81,15 @@ export default class RedisSocket extends EventEmitter { return this.#writableNeedDrain; } - constructor(initiator?: RedisSocketInitiator, options?: RedisSocketOptions) { + constructor( + initiator: RedisSocketInitiator, + returnStringsAsBuffers: ReturnStringsAsBuffers, + options?: RedisSocketOptions + ) { super(); this.#initiator = initiator; + this.#returnStringsAsBuffers = returnStringsAsBuffers; this.#options = RedisSocket.#initiateOptions(options); } @@ -113,23 +120,22 @@ export default class RedisSocket extends EventEmitter { this.emit('connect'); - if (this.#initiator) { - try { - await this.#initiator(); - } catch (err) { - this.#socket.destroy(); - this.#socket = undefined; - if (err instanceof AuthError) { - this.#isOpen = false; - } + try { + await this.#initiator(); + } catch (err) { + this.#socket.destroy(); + this.#socket = undefined; - throw err; + if (err instanceof AuthError) { + this.#isOpen = false; } - if (!this.#isOpen) return; + throw err; } + if (!this.#isOpen) return; + this.#isReady = true; this.emit('ready'); @@ -186,7 +192,10 @@ export default class RedisSocket extends EventEmitter { this.#writableNeedDrain = false; this.emit('drain'); }) - .on('data', (data: Buffer) => this.emit('data', data)); + .pipe(new RESP2Decoder({ + returnStringsAsBuffers: this.#returnStringsAsBuffers + })) + .on('data', (reply: Reply) => this.emit('reply', reply)); resolve(socket); }); diff --git a/packages/client/lib/commander.spec.ts b/packages/client/lib/commander.spec.ts deleted file mode 100644 index f0690f37369..00000000000 --- a/packages/client/lib/commander.spec.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { strict as assert } from 'assert'; -import { describe } from 'mocha'; -import { encodeCommand } from './commander'; - - -describe('Commander', () => { - describe('encodeCommand (see #1628)', () => { - it('1 byte', () => { - assert.deepEqual( - [...encodeCommand(['a', 'z'])], - ['*2\r\n$1\r\na\r\n$1\r\nz\r\n'] - ); - }); - - it('2 bytes', () => { - assert.deepEqual( - [...encodeCommand(['讗', '转'])], - ['*2\r\n$2\r\n讗\r\n$2\r\n转\r\n'] - ); - }); - - it('4 bytes', () => { - assert.deepEqual( - [...encodeCommand(['馃悾', '馃悿'])], - ['*2\r\n$4\r\n馃悾\r\n$4\r\n馃悿\r\n'] - ); - }); - - it('with a buffer', () => { - assert.deepEqual( - [...encodeCommand([Buffer.from('string')])], - ['*1\r\n$6\r\n', Buffer.from('string'), '\r\n'] - ); - }); - }); -}); diff --git a/packages/client/lib/commander.ts b/packages/client/lib/commander.ts index b3dfff0a99f..eca774b049a 100644 --- a/packages/client/lib/commander.ts +++ b/packages/client/lib/commander.ts @@ -89,39 +89,6 @@ export function transformCommandArguments( }; } -const DELIMITER = '\r\n'; - -export function* encodeCommand(args: RedisCommandArguments): IterableIterator { - let strings = `*${args.length}${DELIMITER}`, - stringsLength = 0; - for (const arg of args) { - if (Buffer.isBuffer(arg)) { - yield `${strings}$${arg.length}${DELIMITER}`; - strings = ''; - stringsLength = 0; - yield arg; - } else { - const string = arg?.toString?.() ?? '', - byteLength = Buffer.byteLength(string); - strings += `$${byteLength}${DELIMITER}`; - - const totalLength = stringsLength + byteLength; - if (totalLength > 1024) { - yield strings; - strings = string; - stringsLength = byteLength; - } else { - strings += string; - stringsLength = totalLength; - } - } - - strings += DELIMITER; - } - - yield strings; -} - export function transformCommandReply( command: RedisCommand, rawReply: RedisCommandRawReply, diff --git a/packages/client/lib/test-utils.ts b/packages/client/lib/test-utils.ts index 321a9da63d5..fbed7698896 100644 --- a/packages/client/lib/test-utils.ts +++ b/packages/client/lib/test-utils.ts @@ -44,6 +44,6 @@ export async function waitTillBeenCalled(spy: SinonSpy): Promise { throw new Error('Waiting for more than 1 second'); } - await promiseTimeout(1); + await promiseTimeout(50); } while (spy.callCount === calls); } diff --git a/packages/client/package.json b/packages/client/package.json index 65e6b077f6a..fea751e6f9e 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -8,7 +8,7 @@ "dist/" ], "scripts": { - "test": "nyc -r text-summary -r lcov mocha -r source-map-support/register -r ts-node/register './lib/client/RESP/*.spec.ts'", + "test": "nyc -r text-summary -r lcov mocha -r source-map-support/register -r ts-node/register './lib/**/*.spec.ts'", "build": "tsc", "lint": "eslint ./*.ts ./lib/**/*.ts", "documentation": "typedoc" @@ -16,14 +16,12 @@ "dependencies": { "cluster-key-slot": "1.1.0", "generic-pool": "3.8.2", - "redis-parser": "3.0.0", "yallist": "4.0.0" }, "devDependencies": { "@istanbuljs/nyc-config-typescript": "^1.0.2", "@node-redis/test-utils": "*", "@types/node": "^17.0.13", - "@types/redis-parser": "^3.0.0", "@types/sinon": "^10.0.9", "@types/yallist": "^4.0.1", "@typescript-eslint/eslint-plugin": "^5.10.1", diff --git a/packages/client/tsconfig.json b/packages/client/tsconfig.json index 5e044cbaa1e..3271cf400a2 100644 --- a/packages/client/tsconfig.json +++ b/packages/client/tsconfig.json @@ -11,6 +11,9 @@ "./lib/test-utils.ts", "./lib/**/*.spec.ts" ], + "ts-node": { + "transpileOnly": true + }, "typedocOptions": { "entryPoints": [ "./index.ts", From 9c7087ea0760033af0dcda7a0e9dfbeb420f78e9 Mon Sep 17 00:00:00 2001 From: leibale Date: Mon, 7 Feb 2022 15:16:31 -0500 Subject: [PATCH 03/24] clean code --- packages/client/lib/client/commands-queue.ts | 1 - packages/client/lib/commander.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 8b3e5340afd..587a9119420 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -372,7 +372,6 @@ export default class RedisCommandsQueue { RedisCommandsQueue.#PUB_SUB_MESSAGES.unsubscribe.equals(reply[0]) || RedisCommandsQueue.#PUB_SUB_MESSAGES.pUnsubscribe.equals(reply[0]) ) { - const a = this.#waitingForReply.head!.value as any; if (--this.#waitingForReply.head!.value.channelsCounter! === 0) { this.#waitingForReply.shift()!.resolve(); } diff --git a/packages/client/lib/commander.ts b/packages/client/lib/commander.ts index eca774b049a..d9440682c63 100644 --- a/packages/client/lib/commander.ts +++ b/packages/client/lib/commander.ts @@ -1,6 +1,6 @@ import { CommandOptions, isCommandOptions } from './command-options'; -import { RedisCommand, RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply, RedisCommandReply, RedisCommands, RedisModules, RedisScript, RedisScripts } from './commands'; +import { RedisCommand, RedisCommandArguments, RedisCommandRawReply, RedisCommandReply, RedisCommands, RedisModules, RedisScript, RedisScripts } from './commands'; type Instantiable = new(...args: Array) => T; From 35466e78433a9087109076aa20229347366e3db1 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 9 Feb 2022 13:34:03 -0500 Subject: [PATCH 04/24] fix simple string and bulk string cursor --- packages/client/lib/client/RESP2/decoder.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index 546ddaa3c0a..32eb1c0cf96 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -107,7 +107,7 @@ export default class RESP2Decoder extends Transform { const toWrite = chunk.slice(this.#cursor); composer.write(toWrite); - this.#cursor = toWrite.length; + this.#cursor = chunk.length; } #findCRLF(chunk: Buffer): number { @@ -144,7 +144,6 @@ export default class RESP2Decoder extends Transform { this.#bulkStringRemainingLength = length; - if (this.#cursor >= chunk.length) return; } @@ -161,7 +160,7 @@ export default class RESP2Decoder extends Transform { const toWrite = chunk.slice(this.#cursor); this.#composer.write(toWrite); this.#bulkStringRemainingLength -= toWrite.length; - this.#cursor = toWrite.length; + this.#cursor = chunk.length; } #arraysInProcess: Array = []; From 48fb5ae53c95a1abfd93d33d8c745a12702e36d4 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 9 Feb 2022 17:31:58 -0500 Subject: [PATCH 05/24] performance improvements --- packages/client/lib/client/RESP2/composers/string.ts | 10 +++++----- packages/client/lib/client/RESP2/decoder.ts | 2 +- packages/client/lib/client/RESP2/encoder.ts | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/client/lib/client/RESP2/composers/string.ts b/packages/client/lib/client/RESP2/composers/string.ts index 646318493c2..5e7917eadf2 100644 --- a/packages/client/lib/client/RESP2/composers/string.ts +++ b/packages/client/lib/client/RESP2/composers/string.ts @@ -4,15 +4,15 @@ import { Composer } from './interface'; export default class StringComposer implements Composer { #decoder = new StringDecoder(); - #chunks: Array = []; + #string = ''; write(buffer: Buffer): void { - this.#chunks.push( - this.#decoder.write(buffer) - ); + this.#string += this.#decoder.write(buffer); } end(buffer: Buffer): string { - return this.#chunks.splice(0).join('') + this.#decoder.end(buffer); + const string = this.#string + this.#decoder.end(buffer); + this.#string = ''; + return string; } } diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index 32eb1c0cf96..71b3b3ddebe 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -130,7 +130,7 @@ export default class RESP2Decoder extends Transform { #parseInteger(chunk: Buffer): number | undefined { const number = this.#parseSimpleString(chunk, this.#stringComposer); if (number !== undefined) { - return Number(number); + return parseInt(number); } } diff --git a/packages/client/lib/client/RESP2/encoder.ts b/packages/client/lib/client/RESP2/encoder.ts index 489f14a0ecc..4472573210d 100644 --- a/packages/client/lib/client/RESP2/encoder.ts +++ b/packages/client/lib/client/RESP2/encoder.ts @@ -5,7 +5,8 @@ const CRLF = '\r\n'; export default function* encodeCommand(args: RedisCommandArguments): IterableIterator { let strings = `*${args.length}${CRLF}`, stringsLength = 0; - for (const arg of args) { + for (let i = 0; i < args.length; i++) { + const arg = args[i]; if (Buffer.isBuffer(arg)) { yield `${strings}$${arg.length}${CRLF}`; strings = ''; From 1d09acb2bd9f2e49ef54adb1b8273465794fdb2d Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 9 Feb 2022 17:39:09 -0500 Subject: [PATCH 06/24] change typescript compiler target --- tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tsconfig.json b/tsconfig.json index 285b7ff0a97..0bf4647b24d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,7 @@ { "extends": "./tsconfig.base.json", "compilerOptions": { + "target": "ESNext", "outDir": "./dist" }, "include": [ From 165bee555e057db34b914dfe9b42411d6ef442a6 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 9 Feb 2022 18:25:29 -0500 Subject: [PATCH 07/24] do not use stream.Transform --- .../client/lib/client/RESP2/decoder.spec.ts | 34 ++++++++------- packages/client/lib/client/RESP2/decoder.ts | 13 +++--- packages/client/lib/client/commands-queue.ts | 42 ++++++++++--------- packages/client/lib/client/index.ts | 4 +- packages/client/lib/client/socket.spec.ts | 1 - packages/client/lib/client/socket.ts | 10 +---- 6 files changed, 49 insertions(+), 55 deletions(-) diff --git a/packages/client/lib/client/RESP2/decoder.spec.ts b/packages/client/lib/client/RESP2/decoder.spec.ts index aae42fa7deb..22e2dea8eb6 100644 --- a/packages/client/lib/client/RESP2/decoder.spec.ts +++ b/packages/client/lib/client/RESP2/decoder.spec.ts @@ -3,21 +3,23 @@ import { SinonSpy, spy } from 'sinon'; import RESP2Decoder from './decoder'; import { ErrorReply } from '../../errors'; -interface Stream { - stream: RESP2Decoder; +interface DecoderAndSpies { + decoder: RESP2Decoder; returnStringsAsBuffersSpy: SinonSpy; - repliesSpy: SinonSpy; + onReplySpy: SinonSpy; } -function createStreamAndSpies(returnStringsAsBuffers: boolean): Stream { +function createDecoderAndSpies(returnStringsAsBuffers: boolean): DecoderAndSpies { const returnStringsAsBuffersSpy = spy(() => returnStringsAsBuffers), - repliesSpy = spy(); + onReplySpy = spy(); return { - stream: new RESP2Decoder({ returnStringsAsBuffers: returnStringsAsBuffersSpy }) - .on('data', repliesSpy), + decoder: new RESP2Decoder({ + returnStringsAsBuffers: returnStringsAsBuffersSpy, + onReply: onReplySpy + }), returnStringsAsBuffersSpy, - repliesSpy + onReplySpy }; } @@ -43,19 +45,19 @@ function generateTests({ }: TestsOptions): void { it('single chunk', () => { - const { stream, returnStringsAsBuffersSpy, repliesSpy } = - createStreamAndSpies(returnStringsAsBuffers); - stream.write(toWrite); + const { decoder, returnStringsAsBuffersSpy, onReplySpy } = + createDecoderAndSpies(returnStringsAsBuffers); + decoder.write(toWrite); assert.equal(returnStringsAsBuffersSpy.callCount, replies.length); - testReplies(repliesSpy, replies); + testReplies(onReplySpy, replies); }); it('multiple chunks', () => { - const { stream, returnStringsAsBuffersSpy, repliesSpy } = - createStreamAndSpies(returnStringsAsBuffers); - writeChunks(stream, toWrite); + const { decoder, returnStringsAsBuffersSpy, onReplySpy } = + createDecoderAndSpies(returnStringsAsBuffers); + writeChunks(decoder, toWrite); assert.equal(returnStringsAsBuffersSpy.callCount, replies.length); - testReplies(repliesSpy, replies); + testReplies(onReplySpy, replies); }); } diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index 71b3b3ddebe..a32db63b30f 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -1,4 +1,3 @@ -import { Transform, TransformCallback } from 'stream'; import { ErrorReply } from '../../errors'; import { Composer } from './composers/interface'; import BufferComposer from './composers/buffer'; @@ -21,7 +20,8 @@ type ArrayReply = Array | null; export type ReturnStringsAsBuffers = () => boolean; interface RESP2Options { - returnStringsAsBuffers: ReturnStringsAsBuffers + returnStringsAsBuffers: ReturnStringsAsBuffers; + onReply(reply: Reply): unknown; } interface ArrayInProcess { @@ -29,12 +29,10 @@ interface ArrayInProcess { pushCounter: number; } -export default class RESP2Decoder extends Transform { +export default class RESP2Decoder { #options: RESP2Options; constructor(options: RESP2Options) { - super({ readableObjectMode: true }); - this.#options = options; } @@ -48,7 +46,7 @@ export default class RESP2Decoder extends Transform { #composer: BufferComposer | StringComposer = this.#stringComposer; - _transform(chunk: Buffer, _: BufferEncoding, callback: TransformCallback): void { + write(chunk: Buffer): void { while (this.#cursor < chunk.length) { if (!this.#type) { this.#composer = this.#options.returnStringsAsBuffers() ? @@ -63,11 +61,10 @@ export default class RESP2Decoder extends Transform { if (reply === undefined) break; this.#type = undefined; - this.emit('data', reply); + this.#options.onReply(reply); } this.#cursor -= chunk.length; - callback(); } #parseType(chunk: Buffer, type: Types, arraysToKeep?: number): Reply | undefined { diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 587a9119420..6613a767a40 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -1,7 +1,7 @@ import * as LinkedList from 'yallist'; import { AbortError, ErrorReply } from '../errors'; import { RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply } from '../commands'; -import { Reply } from './RESP2/decoder'; +import RESP2Decoder from './RESP2/decoder'; export interface QueueCommandOptions { asap?: boolean; @@ -106,6 +106,26 @@ export default class RedisCommandsQueue { #chainInExecution: symbol | undefined; + #decoder = new RESP2Decoder({ + returnStringsAsBuffers: () => { + return !!this.#waitingForReply.head?.value.returnBuffers || !!this.#pubSubState; + }, + onReply: reply => { + if (this.#handlePubSubReply(reply)) { + return; + } else if (!this.#waitingForReply.length) { + throw new Error('Got an unexpected reply from Redis'); + } + + const { resolve, reject } = this.#waitingForReply.shift()!; + if (reply instanceof ErrorReply) { + reject(reply); + } else { + resolve(reply); + } + } + }); + constructor(maxLength: number | null | undefined) { this.#maxLength = maxLength; } @@ -330,24 +350,8 @@ export default class RedisCommandsQueue { return toSend?.args; } - returnStringsAsBuffers(): boolean { - return !!this.#waitingForReply.head?.value.returnBuffers || - !!this.#pubSubState; - } - - handleReply(reply: Reply): void { - if (this.#handlePubSubReply(reply)) { - return; - } else if (!this.#waitingForReply.length) { - throw new Error('Got an unexpected reply from Redis'); - } - - const { resolve, reject } = this.#waitingForReply.shift()!; - if (reply instanceof ErrorReply) { - reject(reply); - } else { - resolve(reply); - } + onReplyChunk(chunk: Buffer): void { + this.#decoder.write(chunk); } #handlePubSubReply(reply: any): boolean { diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 326a96bdf28..14afcf0c490 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -274,8 +274,8 @@ export default class RedisClient } }; - return new RedisSocket(socketInitiator, () => this.#queue.returnStringsAsBuffers(), this.#options?.socket) - .on('reply', reply => this.#queue.handleReply(reply)) + return new RedisSocket(socketInitiator, this.#options?.socket) + .on('data', chunk => this.#queue.onReplyChunk(chunk)) .on('error', err => { this.emit('error', err); if (this.#socket.isOpen) { diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 2f30c461dc8..ecdad8a90d1 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -23,7 +23,6 @@ describe('Socket', () => { const socket = new RedisSocket( () => Promise.resolve(), - () => false, { host: 'error', reconnectStrategy diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 2de4407e114..7c9b72b6418 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -5,7 +5,6 @@ import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, AuthError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; import encodeCommand from './RESP2/encoder'; -import RESP2Decoder, { Reply, ReturnStringsAsBuffers } from './RESP2/decoder'; export interface RedisSocketCommonOptions { connectTimeout?: number; noDelay?: boolean; @@ -55,8 +54,6 @@ export default class RedisSocket extends EventEmitter { readonly #initiator: RedisSocketInitiator; - readonly #returnStringsAsBuffers: ReturnStringsAsBuffers; - readonly #options: RedisSocketOptions; #socket?: net.Socket | tls.TLSSocket; @@ -83,13 +80,11 @@ export default class RedisSocket extends EventEmitter { constructor( initiator: RedisSocketInitiator, - returnStringsAsBuffers: ReturnStringsAsBuffers, options?: RedisSocketOptions ) { super(); this.#initiator = initiator; - this.#returnStringsAsBuffers = returnStringsAsBuffers; this.#options = RedisSocket.#initiateOptions(options); } @@ -192,10 +187,7 @@ export default class RedisSocket extends EventEmitter { this.#writableNeedDrain = false; this.emit('drain'); }) - .pipe(new RESP2Decoder({ - returnStringsAsBuffers: this.#returnStringsAsBuffers - })) - .on('data', (reply: Reply) => this.emit('reply', reply)); + .on('data', data => this.emit('data', data)); resolve(socket); }); From ae9fed0a63727f4791ef71ef8e421b6744ebf87b Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Thu, 10 Feb 2022 10:32:34 -0500 Subject: [PATCH 08/24] Update decoder.ts --- packages/client/lib/client/RESP2/decoder.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index a32db63b30f..de4d2848d37 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -3,6 +3,9 @@ import { Composer } from './composers/interface'; import BufferComposer from './composers/buffer'; import StringComposer from './composers/string'; +// RESP2 specification +// https://redis.io/topics/protocol + enum Types { SIMPLE_STRING = 43, // + ERROR = 45, // - From 9953e7ea07729031f072d2b192aa1359e42bfdd9 Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 11:12:15 -0500 Subject: [PATCH 09/24] fix for 1d09acb --- tsconfig.base.json | 1 + tsconfig.json | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.base.json b/tsconfig.base.json index 35daa1df657..304c0b1137b 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -1,6 +1,7 @@ { "extends": "@tsconfig/node12/tsconfig.json", "compilerOptions": { + "target": "ESNext", "declaration": true, "allowJs": true, "useDefineForClassFields": true, diff --git a/tsconfig.json b/tsconfig.json index 0bf4647b24d..285b7ff0a97 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,6 @@ { "extends": "./tsconfig.base.json", "compilerOptions": { - "target": "ESNext", "outDir": "./dist" }, "include": [ From 89415ccfe951941f53d191a4ef9ecc6323a52932 Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 14:08:34 -0500 Subject: [PATCH 10/24] improve integer performance --- .../client/lib/client/RESP2/decoder.spec.ts | 21 +++++--- packages/client/lib/client/RESP2/decoder.ts | 53 ++++++++++++++----- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/packages/client/lib/client/RESP2/decoder.spec.ts b/packages/client/lib/client/RESP2/decoder.spec.ts index 22e2dea8eb6..dcce9f60115 100644 --- a/packages/client/lib/client/RESP2/decoder.spec.ts +++ b/packages/client/lib/client/RESP2/decoder.spec.ts @@ -43,7 +43,6 @@ function generateTests({ returnStringsAsBuffers, replies }: TestsOptions): void { - it('single chunk', () => { const { decoder, returnStringsAsBuffersSpy, onReplySpy } = createDecoderAndSpies(returnStringsAsBuffers); @@ -98,16 +97,26 @@ describe('RESP2Parser', () => { describe('Error', () => { generateTests({ toWrite: Buffer.from('-ERR\r\n'), - returnStringsAsBuffers: true, + returnStringsAsBuffers: false, replies: [[new ErrorReply('ERR')]] }); }); describe('Integer', () => { - generateTests({ - toWrite: Buffer.from(':0\r\n'), - returnStringsAsBuffers: true, - replies: [[0]] + describe('-1', () => { + generateTests({ + toWrite: Buffer.from(':-1\r\n'), + returnStringsAsBuffers: false, + replies: [[-1]] + }); + }); + + describe('0', () => { + generateTests({ + toWrite: Buffer.from(':0\r\n'), + returnStringsAsBuffers: false, + replies: [[0]] + }); }); }); diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index de4d2848d37..2584c397299 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -14,7 +14,11 @@ enum Types { ARRAY = 42 // * } -const CR = 13; // \r +enum ASCII { + CR = 13, // \r + ZERO = 48, + MINUS = 45 +} export type Reply = string | Buffer | ErrorReply | number | null | Array; @@ -47,12 +51,12 @@ export default class RESP2Decoder { #stringComposer = new StringComposer(); - #composer: BufferComposer | StringComposer = this.#stringComposer; + #currentStringComposer: BufferComposer | StringComposer = this.#stringComposer; write(chunk: Buffer): void { while (this.#cursor < chunk.length) { if (!this.#type) { - this.#composer = this.#options.returnStringsAsBuffers() ? + this.#currentStringComposer = this.#options.returnStringsAsBuffers() ? this.#bufferComposer : this.#stringComposer; @@ -73,7 +77,7 @@ export default class RESP2Decoder { #parseType(chunk: Buffer, type: Types, arraysToKeep?: number): Reply | undefined { switch (type) { case Types.SIMPLE_STRING: - return this.#parseSimpleString(chunk, this.#composer); + return this.#parseSimpleString(chunk); case Types.ERROR: return this.#parseError(chunk); @@ -89,13 +93,13 @@ export default class RESP2Decoder { } } - #parseSimpleString< + #compose< C extends Composer, T = C extends Composer ? TT : never >( chunk: Buffer, composer: C - ): T | undefined { + ) { const crIndex = this.#findCRLF(chunk); if (crIndex !== -1) { const reply = composer.end( @@ -112,7 +116,7 @@ export default class RESP2Decoder { #findCRLF(chunk: Buffer): number { for (let i = this.#cursor; i < chunk.length; i++) { - if (chunk[i] === CR) { + if (chunk[i] === ASCII.CR) { return i; } } @@ -120,18 +124,41 @@ export default class RESP2Decoder { return -1; } + #parseSimpleString(chunk: Buffer): string | Buffer | undefined { + return this.#compose(chunk, this.#currentStringComposer); + } + #parseError(chunk: Buffer): ErrorReply | undefined { - const message = this.#parseSimpleString(chunk, this.#stringComposer); + const message = this.#compose(chunk, this.#stringComposer); if (message !== undefined) { return new ErrorReply(message); } } + #integer = 0; + + #isNegativeInteger?: boolean; + #parseInteger(chunk: Buffer): number | undefined { - const number = this.#parseSimpleString(chunk, this.#stringComposer); - if (number !== undefined) { - return parseInt(number); + if (this.#isNegativeInteger === undefined) { + this.#isNegativeInteger = chunk[this.#cursor] === ASCII.MINUS; + if (this.#isNegativeInteger) { + if (++this.#cursor === chunk.length) return; + } } + + do { + const byte = chunk[this.#cursor]; + if (byte === ASCII.CR) { + const integer = this.#isNegativeInteger ? -this.#integer : this.#integer; + this.#integer = 0; + this.#isNegativeInteger = undefined; + this.#cursor += 2; + return integer; + } + + this.#integer = this.#integer * 10 + byte - ASCII.ZERO; + } while (++this.#cursor < chunk.length); } #bulkStringRemainingLength?: number; @@ -149,7 +176,7 @@ export default class RESP2Decoder { const end = this.#cursor + this.#bulkStringRemainingLength; if (chunk.length >= end) { - const reply = this.#composer.end( + const reply = this.#currentStringComposer.end( chunk.slice(this.#cursor, end) ); this.#bulkStringRemainingLength = undefined; @@ -158,7 +185,7 @@ export default class RESP2Decoder { } const toWrite = chunk.slice(this.#cursor); - this.#composer.write(toWrite); + this.#currentStringComposer.write(toWrite); this.#bulkStringRemainingLength -= toWrite.length; this.#cursor = chunk.length; } From 8097431c1cc14e382b890d3d8ec4689b7eb03ee7 Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 14:27:59 -0500 Subject: [PATCH 11/24] revert 1d09acb --- tsconfig.base.json | 1 - 1 file changed, 1 deletion(-) diff --git a/tsconfig.base.json b/tsconfig.base.json index 304c0b1137b..35daa1df657 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -1,7 +1,6 @@ { "extends": "@tsconfig/node12/tsconfig.json", "compilerOptions": { - "target": "ESNext", "declaration": true, "allowJs": true, "useDefineForClassFields": true, From 04e9eca88b30fc509effb17212de56092840aca1 Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 14:50:37 -0500 Subject: [PATCH 12/24] improve RESP2 decoder performance --- packages/client/lib/client/RESP2/decoder.ts | 205 +++++++++----------- 1 file changed, 97 insertions(+), 108 deletions(-) diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index 2584c397299..26822f87e0c 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -36,222 +36,211 @@ interface ArrayInProcess { pushCounter: number; } -export default class RESP2Decoder { - #options: RESP2Options; +// Using TypeScript `private` and not the build-in `#` to avoid __classPrivateFieldGet and __classPrivateFieldSet - constructor(options: RESP2Options) { - this.#options = options; - } +export default class RESP2Decoder { + constructor(private options: RESP2Options) {} - #cursor = 0; + private cursor = 0; - #type?: Types; + private type?: Types; - #bufferComposer = new BufferComposer(); + private bufferComposer = new BufferComposer(); - #stringComposer = new StringComposer(); + private stringComposer = new StringComposer(); - #currentStringComposer: BufferComposer | StringComposer = this.#stringComposer; + private currentStringComposer: BufferComposer | StringComposer = this.stringComposer; write(chunk: Buffer): void { - while (this.#cursor < chunk.length) { - if (!this.#type) { - this.#currentStringComposer = this.#options.returnStringsAsBuffers() ? - this.#bufferComposer : - this.#stringComposer; - - this.#type = chunk[this.#cursor]; - if (++this.#cursor >= chunk.length) break; + while (this.cursor < chunk.length) { + if (!this.type) { + this.currentStringComposer = this.options.returnStringsAsBuffers() ? + this.bufferComposer : + this.stringComposer; + + this.type = chunk[this.cursor]; + if (++this.cursor >= chunk.length) break; } - const reply = this.#parseType(chunk, this.#type); + const reply = this.parseType(chunk, this.type); if (reply === undefined) break; - this.#type = undefined; - this.#options.onReply(reply); + this.type = undefined; + this.options.onReply(reply); } - this.#cursor -= chunk.length; + this.cursor -= chunk.length; } - #parseType(chunk: Buffer, type: Types, arraysToKeep?: number): Reply | undefined { + private parseType(chunk: Buffer, type: Types, arraysToKeep?: number): Reply | undefined { switch (type) { case Types.SIMPLE_STRING: - return this.#parseSimpleString(chunk); + return this.parseSimpleString(chunk); case Types.ERROR: - return this.#parseError(chunk); + return this.parseError(chunk); case Types.INTEGER: - return this.#parseInteger(chunk); + return this.parseInteger(chunk); case Types.BULK_STRING: - return this.#parseBulkString(chunk); + return this.parseBulkString(chunk); case Types.ARRAY: - return this.#parseArray(chunk, arraysToKeep); + return this.parseArray(chunk, arraysToKeep); } } - #compose< + private compose< C extends Composer, T = C extends Composer ? TT : never >( chunk: Buffer, composer: C - ) { - const crIndex = this.#findCRLF(chunk); - if (crIndex !== -1) { - const reply = composer.end( - chunk.slice(this.#cursor, crIndex) - ); - this.#cursor = crIndex + 2; - return reply; - } - - const toWrite = chunk.slice(this.#cursor); - composer.write(toWrite); - this.#cursor = chunk.length; - } - - #findCRLF(chunk: Buffer): number { - for (let i = this.#cursor; i < chunk.length; i++) { + ): T | undefined { + for (let i = this.cursor; i < chunk.length; i++) { if (chunk[i] === ASCII.CR) { - return i; + const reply = composer.end( + chunk.slice(this.cursor, i) + ); + this.cursor = i + 2; + return reply; } } - return -1; + const toWrite = chunk.slice(this.cursor); + composer.write(toWrite); + this.cursor = chunk.length; } - #parseSimpleString(chunk: Buffer): string | Buffer | undefined { - return this.#compose(chunk, this.#currentStringComposer); + private parseSimpleString(chunk: Buffer): string | Buffer | undefined { + return this.compose(chunk, this.currentStringComposer); } - #parseError(chunk: Buffer): ErrorReply | undefined { - const message = this.#compose(chunk, this.#stringComposer); + private parseError(chunk: Buffer): ErrorReply | undefined { + const message = this.compose(chunk, this.stringComposer); if (message !== undefined) { return new ErrorReply(message); } } - #integer = 0; + private integer = 0; - #isNegativeInteger?: boolean; + private isNegativeInteger?: boolean; - #parseInteger(chunk: Buffer): number | undefined { - if (this.#isNegativeInteger === undefined) { - this.#isNegativeInteger = chunk[this.#cursor] === ASCII.MINUS; - if (this.#isNegativeInteger) { - if (++this.#cursor === chunk.length) return; + private parseInteger(chunk: Buffer): number | undefined { + if (this.isNegativeInteger === undefined) { + this.isNegativeInteger = chunk[this.cursor] === ASCII.MINUS; + if (this.isNegativeInteger) { + if (++this.cursor === chunk.length) return; } } do { - const byte = chunk[this.#cursor]; + const byte = chunk[this.cursor]; if (byte === ASCII.CR) { - const integer = this.#isNegativeInteger ? -this.#integer : this.#integer; - this.#integer = 0; - this.#isNegativeInteger = undefined; - this.#cursor += 2; + const integer = this.isNegativeInteger ? -this.integer : this.integer; + this.integer = 0; + this.isNegativeInteger = undefined; + this.cursor += 2; return integer; } - this.#integer = this.#integer * 10 + byte - ASCII.ZERO; - } while (++this.#cursor < chunk.length); + this.integer = this.integer * 10 + byte - ASCII.ZERO; + } while (++this.cursor < chunk.length); } - #bulkStringRemainingLength?: number; + private bulkStringRemainingLength?: number; - #parseBulkString(chunk: Buffer): string | Buffer | null | undefined { - if (this.#bulkStringRemainingLength === undefined) { - const length = this.#parseInteger(chunk); + private parseBulkString(chunk: Buffer): string | Buffer | null | undefined { + if (this.bulkStringRemainingLength === undefined) { + const length = this.parseInteger(chunk); if (length === undefined) return; else if (length === -1) return null; - this.#bulkStringRemainingLength = length; + this.bulkStringRemainingLength = length; - if (this.#cursor >= chunk.length) return; + if (this.cursor >= chunk.length) return; } - const end = this.#cursor + this.#bulkStringRemainingLength; + const end = this.cursor + this.bulkStringRemainingLength; if (chunk.length >= end) { - const reply = this.#currentStringComposer.end( - chunk.slice(this.#cursor, end) + const reply = this.currentStringComposer.end( + chunk.slice(this.cursor, end) ); - this.#bulkStringRemainingLength = undefined; - this.#cursor = end + 2; + this.bulkStringRemainingLength = undefined; + this.cursor = end + 2; return reply; } - const toWrite = chunk.slice(this.#cursor); - this.#currentStringComposer.write(toWrite); - this.#bulkStringRemainingLength -= toWrite.length; - this.#cursor = chunk.length; + const toWrite = chunk.slice(this.cursor); + this.currentStringComposer.write(toWrite); + this.bulkStringRemainingLength -= toWrite.length; + this.cursor = chunk.length; } - #arraysInProcess: Array = []; + private arraysInProcess: Array = []; - #initializeArray = false; + private initializeArray = false; - #arrayItemType?: Types; + private arrayItemType?: Types; - #parseArray(chunk: Buffer, arraysToKeep = 0): ArrayReply | undefined { - if (this.#initializeArray || this.#arraysInProcess.length === arraysToKeep) { - const length = this.#parseInteger(chunk); + private parseArray(chunk: Buffer, arraysToKeep = 0): ArrayReply | undefined { + if (this.initializeArray || this.arraysInProcess.length === arraysToKeep) { + const length = this.parseInteger(chunk); if (length === undefined) { - this.#initializeArray = true; + this.initializeArray = true; return undefined; } - this.#initializeArray = false; - this.#arrayItemType = undefined; + this.initializeArray = false; + this.arrayItemType = undefined; if (length === -1) { - return this.#returnArrayReply(null, arraysToKeep); + return this.returnArrayReply(null, arraysToKeep); } else if (length === 0) { - return this.#returnArrayReply([], arraysToKeep); + return this.returnArrayReply([], arraysToKeep); } - this.#arraysInProcess.push({ + this.arraysInProcess.push({ array: new Array(length), pushCounter: 0 }); } - while (this.#cursor < chunk.length) { - if (!this.#arrayItemType) { - this.#arrayItemType = chunk[this.#cursor]; + while (this.cursor < chunk.length) { + if (!this.arrayItemType) { + this.arrayItemType = chunk[this.cursor]; - if (++this.#cursor >= chunk.length) break; + if (++this.cursor >= chunk.length) break; } - const item = this.#parseType( + const item = this.parseType( chunk, - this.#arrayItemType, + this.arrayItemType, arraysToKeep + 1 ); if (item === undefined) break; - this.#arrayItemType = undefined; + this.arrayItemType = undefined; - const reply = this.#pushArrayItem(item, arraysToKeep); + const reply = this.pushArrayItem(item, arraysToKeep); if (reply !== undefined) return reply; } } - #returnArrayReply(reply: ArrayReply, arraysToKeep: number): ArrayReply | undefined { - if (this.#arraysInProcess.length <= arraysToKeep) return reply; + private returnArrayReply(reply: ArrayReply, arraysToKeep: number): ArrayReply | undefined { + if (this.arraysInProcess.length <= arraysToKeep) return reply; - return this.#pushArrayItem(reply, arraysToKeep); + return this.pushArrayItem(reply, arraysToKeep); } - #pushArrayItem(item: Reply, arraysToKeep: number): ArrayReply | undefined { - const to = this.#arraysInProcess[this.#arraysInProcess.length - 1]!; + private pushArrayItem(item: Reply, arraysToKeep: number): ArrayReply | undefined { + const to = this.arraysInProcess[this.arraysInProcess.length - 1]!; to.array[to.pushCounter] = item; if (++to.pushCounter === to.array.length) { - return this.#returnArrayReply( - this.#arraysInProcess.pop()!.array, + return this.returnArrayReply( + this.arraysInProcess.pop()!.array, arraysToKeep ); } From f8dacb743b4cce9767e5e20a4924e45672cf9dfc Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 14:55:36 -0500 Subject: [PATCH 13/24] improve performance --- packages/client/lib/client/RESP2/composers/buffer.ts | 6 +++--- packages/client/lib/client/RESP2/composers/string.ts | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/client/lib/client/RESP2/composers/buffer.ts b/packages/client/lib/client/RESP2/composers/buffer.ts index 9ef8a95835a..eefec0aec89 100644 --- a/packages/client/lib/client/RESP2/composers/buffer.ts +++ b/packages/client/lib/client/RESP2/composers/buffer.ts @@ -1,14 +1,14 @@ import { Composer } from './interface'; export default class BufferComposer implements Composer { - #chunks: Array = []; + private chunks: Array = []; write(buffer: Buffer): void { - this.#chunks.push(buffer); + this.chunks.push(buffer); } end(buffer: Buffer): Buffer { this.write(buffer); - return Buffer.concat(this.#chunks.splice(0)); + return Buffer.concat(this.chunks.splice(0)); } } diff --git a/packages/client/lib/client/RESP2/composers/string.ts b/packages/client/lib/client/RESP2/composers/string.ts index 5e7917eadf2..8684538b45e 100644 --- a/packages/client/lib/client/RESP2/composers/string.ts +++ b/packages/client/lib/client/RESP2/composers/string.ts @@ -2,17 +2,17 @@ import { StringDecoder } from 'string_decoder'; import { Composer } from './interface'; export default class StringComposer implements Composer { - #decoder = new StringDecoder(); + private decoder = new StringDecoder(); - #string = ''; + private string = ''; write(buffer: Buffer): void { - this.#string += this.#decoder.write(buffer); + this.string += this.decoder.write(buffer); } end(buffer: Buffer): string { - const string = this.#string + this.#decoder.end(buffer); - this.#string = ''; + const string = this.string + this.decoder.end(buffer); + this.string = ''; return string; } } From 1331698c6ed1eb8d295319d03962976da314e37d Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 15:58:22 -0500 Subject: [PATCH 14/24] improve encode performance --- .../client/lib/client/RESP2/encoder.spec.ts | 6 +-- packages/client/lib/client/RESP2/encoder.ts | 37 ++++++++----------- packages/client/lib/client/commands-queue.ts | 34 +++++++++++------ packages/client/lib/client/index.spec.ts | 2 +- packages/client/lib/client/index.ts | 4 +- packages/client/lib/client/multi-command.ts | 4 +- packages/client/lib/client/socket.ts | 4 +- packages/client/lib/commander.ts | 4 ++ 8 files changed, 53 insertions(+), 42 deletions(-) diff --git a/packages/client/lib/client/RESP2/encoder.spec.ts b/packages/client/lib/client/RESP2/encoder.spec.ts index c1d0002391f..486259472a4 100644 --- a/packages/client/lib/client/RESP2/encoder.spec.ts +++ b/packages/client/lib/client/RESP2/encoder.spec.ts @@ -5,14 +5,14 @@ import encodeCommand from './encoder'; describe('RESP2 Encoder', () => { it('1 byte', () => { assert.deepEqual( - [...encodeCommand(['a', 'z'])], + encodeCommand(['a', 'z']), ['*2\r\n$1\r\na\r\n$1\r\nz\r\n'] ); }); it('2 bytes', () => { assert.deepEqual( - [...encodeCommand(['讗', '转'])], + encodeCommand(['讗', '转']), ['*2\r\n$2\r\n讗\r\n$2\r\n转\r\n'] ); }); @@ -26,7 +26,7 @@ describe('RESP2 Encoder', () => { it('buffer', () => { assert.deepEqual( - [...encodeCommand([Buffer.from('string')])], + encodeCommand([Buffer.from('string')]), ['*1\r\n$6\r\n', Buffer.from('string'), '\r\n'] ); }); diff --git a/packages/client/lib/client/RESP2/encoder.ts b/packages/client/lib/client/RESP2/encoder.ts index 4472573210d..be48348a356 100644 --- a/packages/client/lib/client/RESP2/encoder.ts +++ b/packages/client/lib/client/RESP2/encoder.ts @@ -2,34 +2,29 @@ import { RedisCommandArgument, RedisCommandArguments } from '../../commands'; const CRLF = '\r\n'; -export default function* encodeCommand(args: RedisCommandArguments): IterableIterator { - let strings = `*${args.length}${CRLF}`, - stringsLength = 0; +export default function encodeCommand(args: RedisCommandArguments): Array { + const toWrite: Array = []; + + let strings = `*${args.length}${CRLF}`; + for (let i = 0; i < args.length; i++) { const arg = args[i]; - if (Buffer.isBuffer(arg)) { - yield `${strings}$${arg.length}${CRLF}`; + if (typeof arg === 'string') { + const byteLength = Buffer.byteLength(arg); + strings += `$${byteLength}${CRLF}`; + strings += arg; + } else if (arg instanceof Buffer) { + toWrite.push(`${strings}$${arg.length}${CRLF}`); strings = ''; - stringsLength = 0; - yield arg; + toWrite.push(arg); } else { - const string = arg?.toString?.() ?? '', - byteLength = Buffer.byteLength(string); - strings += `$${byteLength}${CRLF}`; - - const totalLength = stringsLength + byteLength; - if (totalLength > 1024) { - yield strings; - strings = string; - stringsLength = byteLength; - } else { - strings += string; - stringsLength = totalLength; - } + throw new TypeError('Invalid argument type'); } strings += CRLF; } - yield strings; + toWrite.push(strings); + + return toWrite; } diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 6613a767a40..4599936daf9 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -1,7 +1,8 @@ import * as LinkedList from 'yallist'; import { AbortError, ErrorReply } from '../errors'; -import { RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply } from '../commands'; +import { RedisCommand, RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply } from '../commands'; import RESP2Decoder from './RESP2/decoder'; +import encodeCommand from './RESP2/encoder'; export interface QueueCommandOptions { asap?: boolean; @@ -337,17 +338,28 @@ export default class RedisCommandsQueue { getCommandToSend(): RedisCommandArguments | undefined { const toSend = this.#waitingToBeSent.shift(); - if (toSend) { - this.#waitingForReply.push({ - args: toSend.args, - resolve: toSend.resolve, - reject: toSend.reject, - channelsCounter: toSend.channelsCounter, - returnBuffers: toSend.returnBuffers - } as any); + if (!toSend) return; + + let encoded: RedisCommandArguments; + try { + encoded = encodeCommand(toSend.args); + } catch (err) { + toSend.reject(err); + return; } - this.#chainInExecution = toSend?.chainId; - return toSend?.args; + + this.#waitingForReply.push({ + resolve: toSend.resolve, + reject: toSend.reject, + channelsCounter: toSend.channelsCounter, + returnBuffers: toSend.returnBuffers + }); + this.#chainInExecution = toSend.chainId; + return encoded; + } + + rejectLastCommand(err: unknown): void { + this.#waitingForReply.pop()!.reject(err); } onReplyChunk(chunk: Buffer): void { diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 1d627756c6d..7c0c4ae4952 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -357,7 +357,7 @@ describe('Client', () => { testUtils.testWithClient('undefined and null should not break the client', async client => { await assert.rejects( client.sendCommand([null as any, undefined as any]), - 'ERR unknown command ``, with args beginning with: ``' + TypeError ); assert.equal( diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 14afcf0c490..183baa5468e 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -9,7 +9,7 @@ import { CommandOptions, commandOptions, isCommandOptions } from '../command-opt import { ScanOptions, ZMember } from '../commands/generic-transformers'; import { ScanCommandOptions } from '../commands/SCAN'; import { HScanTuple } from '../commands/HSCAN'; -import { extendWithCommands, extendWithModulesAndScripts, transformCommandArguments, transformCommandReply } from '../commander'; +import { extendWithCommands, extendWithModulesAndScripts, transformCommandArguments, transformCommandReply, transformLegacyCommandArguments } from '../commander'; import { Pool, Options as PoolOptions, createPool } from 'generic-pool'; import { ClientClosedError, DisconnectsClientError, AuthError } from '../errors'; import { URL } from 'url'; @@ -304,7 +304,7 @@ export default class RedisClient callback = args.pop() as ClientLegacyCallback; } - this.#sendCommand(args.flat()) + this.#sendCommand(transformLegacyCommandArguments(args)) .then((reply: RedisCommandRawReply) => { if (!callback) return; diff --git a/packages/client/lib/client/multi-command.ts b/packages/client/lib/client/multi-command.ts index cce0b515f1f..7bc046f29c5 100644 --- a/packages/client/lib/client/multi-command.ts +++ b/packages/client/lib/client/multi-command.ts @@ -1,7 +1,7 @@ import COMMANDS from './commands'; import { RedisCommand, RedisCommandArguments, RedisCommandRawReply, RedisModules, RedisPlugins, RedisScript, RedisScripts } from '../commands'; import RedisMultiCommand, { RedisMultiQueuedCommand } from '../multi-command'; -import { extendWithCommands, extendWithModulesAndScripts } from '../commander'; +import { extendWithCommands, extendWithModulesAndScripts, transformLegacyCommandArguments } from '../commander'; import { ExcludeMappedString } from '.'; type RedisClientMultiCommandSignature = @@ -54,7 +54,7 @@ export default class RedisClientMultiCommand { #legacyMode(): void { this.v4.addCommand = this.addCommand.bind(this); (this as any).addCommand = (...args: Array): this => { - this.#multi.addCommand(args.flat()); + this.#multi.addCommand(transformLegacyCommandArguments(args)); return this; }; this.v4.exec = this.exec.bind(this); diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 7c9b72b6418..75d68b33ccc 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -4,7 +4,7 @@ import * as tls from 'tls'; import { RedisCommandArguments } from '../commands'; import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, AuthError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; -import encodeCommand from './RESP2/encoder'; + export interface RedisSocketCommonOptions { connectTimeout?: number; noDelay?: boolean; @@ -222,7 +222,7 @@ export default class RedisSocket extends EventEmitter { throw new ClientClosedError(); } - for (const toWrite of encodeCommand(args)) { + for (const toWrite of args) { this.#writableNeedDrain = !this.#socket.write(toWrite); } } diff --git a/packages/client/lib/commander.ts b/packages/client/lib/commander.ts index d9440682c63..d70435e14ad 100644 --- a/packages/client/lib/commander.ts +++ b/packages/client/lib/commander.ts @@ -89,6 +89,10 @@ export function transformCommandArguments( }; } +export function transformLegacyCommandArguments(args: Array): Array { + return args.flat().map(x => x?.toString?.()); +} + export function transformCommandReply( command: RedisCommand, rawReply: RedisCommandRawReply, From dcdceb187aae7d759cc344bbb7cb132f9700afa1 Mon Sep 17 00:00:00 2001 From: leibale Date: Thu, 10 Feb 2022 16:12:35 -0500 Subject: [PATCH 15/24] remove unused import --- packages/client/lib/client/commands-queue.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 4599936daf9..05bc9c2b2e9 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -1,6 +1,6 @@ import * as LinkedList from 'yallist'; import { AbortError, ErrorReply } from '../errors'; -import { RedisCommand, RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply } from '../commands'; +import { RedisCommandArgument, RedisCommandArguments, RedisCommandRawReply } from '../commands'; import RESP2Decoder from './RESP2/decoder'; import encodeCommand from './RESP2/encoder'; From 783f4e7ced5c6799745dda875362b2b19cb592c9 Mon Sep 17 00:00:00 2001 From: leibale Date: Mon, 4 Apr 2022 13:14:31 +0300 Subject: [PATCH 16/24] upgrade benchmark deps --- benchmark/package-lock.json | 274 ++++++++++++++---------------------- benchmark/package.json | 6 +- 2 files changed, 111 insertions(+), 169 deletions(-) diff --git a/benchmark/package-lock.json b/benchmark/package-lock.json index af72486e0af..3d67f1d3972 100644 --- a/benchmark/package-lock.json +++ b/benchmark/package-lock.json @@ -6,72 +6,56 @@ "": { "name": "@node-redis/client-benchmark", "dependencies": { - "@node-redis/client-local": "../packages/client", - "@node-redis/client-production": "npm:@node-redis/client@1.0.0", - "hdr-histogram-js": "^2.0.1", - "ioredis": "4.28.1", + "@node-redis/client": "../packages/client", + "hdr-histogram-js": "3.0.0", + "ioredis": "5.0.3", "redis-v3": "npm:redis@3.1.2", - "yargs": "17.3.0" + "yargs": "17.4.0" } }, "../packages/client": { - "name": "@node-redis/client", - "version": "1.0.0", + "version": "1.0.5", "license": "MIT", "dependencies": { "cluster-key-slot": "1.1.0", "generic-pool": "3.8.2", - "redis-parser": "3.0.0", "yallist": "4.0.0" }, "devDependencies": { - "@istanbuljs/nyc-config-typescript": "^1.0.1", + "@istanbuljs/nyc-config-typescript": "^1.0.2", "@node-redis/test-utils": "*", - "@types/node": "^16.11.10", - "@types/redis-parser": "^3.0.0", - "@types/sinon": "^10.0.6", + "@types/node": "^17.0.23", + "@types/sinon": "^10.0.11", "@types/yallist": "^4.0.1", - "@typescript-eslint/eslint-plugin": "^5.4.0", - "@typescript-eslint/parser": "^5.4.0", - "eslint": "^8.3.0", + "@typescript-eslint/eslint-plugin": "^5.16.0", + "@typescript-eslint/parser": "^5.16.0", + "eslint": "^8.12.0", "nyc": "^15.1.0", - "release-it": "^14.11.8", - "sinon": "^12.0.1", + "release-it": "^14.13.1", + "sinon": "^13.0.1", "source-map-support": "^0.5.21", - "ts-node": "^10.4.0", - "typedoc": "^0.22.10", - "typedoc-github-wiki-theme": "^0.6.0", - "typedoc-plugin-markdown": "^3.11.7", - "typescript": "^4.5.2" + "ts-node": "^10.7.0", + "typedoc": "^0.22.13", + "typescript": "^4.6.3" }, "engines": { "node": ">=12" } }, "node_modules/@assemblyscript/loader": { - "version": "0.10.1", - "resolved": "https://registry.npmjs.org/@assemblyscript/loader/-/loader-0.10.1.tgz", - "integrity": "sha512-H71nDOOL8Y7kWRLqf6Sums+01Q5msqBW2KhDUTemh1tvY04eSkSXrK0uj/4mmY0Xr16/3zyZmsrxN7CKuRbNRg==" + "version": "0.19.23", + "resolved": "https://registry.npmjs.org/@assemblyscript/loader/-/loader-0.19.23.tgz", + "integrity": "sha512-ulkCYfFbYj01ie1MDOyxv2F6SpRN1TOj7fQxbP07D6HmeR+gr2JLSmINKjga2emB+b1L2KGrFKBTc+e00p54nw==" }, - "node_modules/@node-redis/client-local": { + "node_modules/@ioredis/commands": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-1.1.1.tgz", + "integrity": "sha512-fsR4P/ROllzf/7lXYyElUJCheWdTJVJvOTps8v9IWKFATxR61ANOlnoPqhH099xYLrJGpc2ZQ28B3rMeUt5VQg==" + }, + "node_modules/@node-redis/client": { "resolved": "../packages/client", "link": true }, - "node_modules/@node-redis/client-production": { - "name": "@node-redis/client", - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/@node-redis/client/-/client-1.0.0.tgz", - "integrity": "sha512-DWDMeZELXG3rOGzCKfJEHCkCP6rgiA1H+oqj2N0NR4Q0fQUYMxTsyoqt80GpdYLilUW6zoCiQl9yL3vJhGhiCA==", - "dependencies": { - "cluster-key-slot": "1.1.0", - "generic-pool": "3.8.2", - "redis-parser": "3.0.0", - "yallist": "4.0.0" - }, - "engines": { - "node": ">=12" - } - }, "node_modules/ansi-regex": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz", @@ -148,9 +132,9 @@ "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==" }, "node_modules/debug": { - "version": "4.3.3", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.3.tgz", - "integrity": "sha512-/zxw5+vh1Tfv+4Qn7a5nsbcJKPaSvCDhojn6FEl9vupwK2VCSDtEiEtqr8DFtzYFOdz63LBkxec7DYuc2jon6Q==", + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", "dependencies": { "ms": "2.1.2" }, @@ -164,9 +148,9 @@ } }, "node_modules/denque": { - "version": "1.5.1", - "resolved": "https://registry.npmjs.org/denque/-/denque-1.5.1.tgz", - "integrity": "sha512-XwE+iZ4D6ZUB7mfYRMb5wByE8L74HCn30FBN7sWnXksWc1LO1bPDl67pBR9o/kC4z/xSNAwkMYcGgqDV3BE3Hw==", + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz", + "integrity": "sha512-tfiWc6BQLXNLpNiR5iGd0Ocu3P3VpxfzFiqubLgMfhfOw9WyvgJBd46CClNn9k3qfbjvT//0cf7AlYRX/OslMQ==", "engines": { "node": ">=0.10" } @@ -184,14 +168,6 @@ "node": ">=6" } }, - "node_modules/generic-pool": { - "version": "3.8.2", - "resolved": "https://registry.npmjs.org/generic-pool/-/generic-pool-3.8.2.tgz", - "integrity": "sha512-nGToKy6p3PAbYQ7p1UlWl6vSPwfwU6TMSWK7TTu+WUY4ZjyZQGniGGt2oNVvyNSpyZYSB43zMXVLcBm08MTMkg==", - "engines": { - "node": ">= 4" - } - }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -201,34 +177,35 @@ } }, "node_modules/hdr-histogram-js": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/hdr-histogram-js/-/hdr-histogram-js-2.0.1.tgz", - "integrity": "sha512-uPZxl1dAFnjUFHWLZmt93vUUvtHeaBay9nVNHu38SdOjMSF/4KqJUqa1Seuj08ptU1rEb6AHvB41X8n/zFZ74Q==", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/hdr-histogram-js/-/hdr-histogram-js-3.0.0.tgz", + "integrity": "sha512-/EpvQI2/Z98mNFYEnlqJ8Ogful8OpArLG/6Tf2bPnkutBVLIeMVNHjk1ZDfshF2BUweipzbk+dB1hgSB7SIakw==", "dependencies": { - "@assemblyscript/loader": "^0.10.1", + "@assemblyscript/loader": "^0.19.21", "base64-js": "^1.2.0", "pako": "^1.0.3" + }, + "engines": { + "node": ">=14" } }, "node_modules/ioredis": { - "version": "4.28.1", - "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-4.28.1.tgz", - "integrity": "sha512-7gcrUJEcPHWy+eEyq6wIZpXtfHt8crhbc5+z0sqrnHUkwBblXinygfamj+/jx83Qo+2LW3q87Nj2VsuH6BF2BA==", + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-5.0.3.tgz", + "integrity": "sha512-hKxywVypoGGUJDyfnMufO0VNkuIaQufo2/PJ5SOYBKWgf4+gCnZEPaBPle615k08euY9/B9DKytVtwZROF7HaA==", "dependencies": { + "@ioredis/commands": "^1.1.1", "cluster-key-slot": "^1.1.0", - "debug": "^4.3.1", - "denque": "^1.1.0", + "debug": "^4.3.4", + "denque": "^2.0.1", "lodash.defaults": "^4.2.0", - "lodash.flatten": "^4.4.0", "lodash.isarguments": "^3.1.0", - "p-map": "^2.1.0", - "redis-commands": "1.7.0", "redis-errors": "^1.2.0", "redis-parser": "^3.0.0", "standard-as-callback": "^2.1.0" }, "engines": { - "node": ">=6" + "node": ">=12.22.0" }, "funding": { "type": "opencollective", @@ -248,11 +225,6 @@ "resolved": "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz", "integrity": "sha1-0JF4cW/+pN3p5ft7N/bwgCJ0WAw=" }, - "node_modules/lodash.flatten": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/lodash.flatten/-/lodash.flatten-4.4.0.tgz", - "integrity": "sha1-8xwiIlqWMtK7+OSt2+8kCqdlph8=" - }, "node_modules/lodash.isarguments": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz", @@ -263,14 +235,6 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, - "node_modules/p-map": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/p-map/-/p-map-2.1.0.tgz", - "integrity": "sha512-y3b8Kpd8OAN444hxfBbFfj1FY/RjtTd8tzYwhUqNYXx0fXx2iX4maP4Qr6qhIKbQXI02wTLAda4fYUbDagTUFw==", - "engines": { - "node": ">=6" - } - }, "node_modules/pako": { "version": "1.0.11", "resolved": "https://registry.npmjs.org/pako/-/pako-1.0.11.tgz", @@ -319,6 +283,14 @@ "url": "https://opencollective.com/node-redis" } }, + "node_modules/redis-v3/node_modules/denque": { + "version": "1.5.1", + "resolved": "https://registry.npmjs.org/denque/-/denque-1.5.1.tgz", + "integrity": "sha512-XwE+iZ4D6ZUB7mfYRMb5wByE8L74HCn30FBN7sWnXksWc1LO1bPDl67pBR9o/kC4z/xSNAwkMYcGgqDV3BE3Hw==", + "engines": { + "node": ">=0.10" + } + }, "node_modules/require-directory": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/require-directory/-/require-directory-2.1.1.tgz", @@ -380,15 +352,10 @@ "node": ">=10" } }, - "node_modules/yallist": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", - "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==" - }, "node_modules/yargs": { - "version": "17.3.0", - "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.3.0.tgz", - "integrity": "sha512-GQl1pWyDoGptFPJx9b9L6kmR33TGusZvXIZUT+BOz9f7X2L94oeAskFYLEg/FkhV06zZPBYLvLZRWeYId29lew==", + "version": "17.4.0", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.4.0.tgz", + "integrity": "sha512-WJudfrk81yWFSOkZYpAZx4Nt7V4xp7S/uJkX0CnxovMCt1wCE8LNftPpNuF9X/u9gN5nsD7ycYtRcDf2pL3UiA==", "dependencies": { "cliui": "^7.0.2", "escalade": "^3.1.1", @@ -403,9 +370,9 @@ } }, "node_modules/yargs-parser": { - "version": "21.0.0", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-21.0.0.tgz", - "integrity": "sha512-z9kApYUOCwoeZ78rfRYYWdiU/iNL6mwwYlkkZfJoyMR1xps+NEBX5X7XmRpxkZHhXJ6+Ey00IwKxBBSW9FIjyA==", + "version": "21.0.1", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-21.0.1.tgz", + "integrity": "sha512-9BK1jFpLzJROCI5TzwZL/TU4gqjK5xiHV/RfWLOahrjAko/e4DJkRDZQXfvqAsiZzzYhgAzbgz6lg48jcm4GLg==", "engines": { "node": ">=12" } @@ -413,45 +380,35 @@ }, "dependencies": { "@assemblyscript/loader": { - "version": "0.10.1", - "resolved": "https://registry.npmjs.org/@assemblyscript/loader/-/loader-0.10.1.tgz", - "integrity": "sha512-H71nDOOL8Y7kWRLqf6Sums+01Q5msqBW2KhDUTemh1tvY04eSkSXrK0uj/4mmY0Xr16/3zyZmsrxN7CKuRbNRg==" + "version": "0.19.23", + "resolved": "https://registry.npmjs.org/@assemblyscript/loader/-/loader-0.19.23.tgz", + "integrity": "sha512-ulkCYfFbYj01ie1MDOyxv2F6SpRN1TOj7fQxbP07D6HmeR+gr2JLSmINKjga2emB+b1L2KGrFKBTc+e00p54nw==" + }, + "@ioredis/commands": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-1.1.1.tgz", + "integrity": "sha512-fsR4P/ROllzf/7lXYyElUJCheWdTJVJvOTps8v9IWKFATxR61ANOlnoPqhH099xYLrJGpc2ZQ28B3rMeUt5VQg==" }, - "@node-redis/client-local": { + "@node-redis/client": { "version": "file:../packages/client", "requires": { - "@istanbuljs/nyc-config-typescript": "^1.0.1", + "@istanbuljs/nyc-config-typescript": "^1.0.2", "@node-redis/test-utils": "*", - "@types/node": "^16.11.10", - "@types/redis-parser": "^3.0.0", - "@types/sinon": "^10.0.6", + "@types/node": "^17.0.23", + "@types/sinon": "^10.0.11", "@types/yallist": "^4.0.1", - "@typescript-eslint/eslint-plugin": "^5.4.0", - "@typescript-eslint/parser": "^5.4.0", + "@typescript-eslint/eslint-plugin": "^5.16.0", + "@typescript-eslint/parser": "^5.16.0", "cluster-key-slot": "1.1.0", - "eslint": "^8.3.0", + "eslint": "^8.12.0", "generic-pool": "3.8.2", "nyc": "^15.1.0", - "redis-parser": "3.0.0", - "release-it": "^14.11.8", - "sinon": "^12.0.1", + "release-it": "^14.13.1", + "sinon": "^13.0.1", "source-map-support": "^0.5.21", - "ts-node": "^10.4.0", - "typedoc": "^0.22.10", - "typedoc-github-wiki-theme": "^0.6.0", - "typedoc-plugin-markdown": "^3.11.7", - "typescript": "^4.5.2", - "yallist": "4.0.0" - } - }, - "@node-redis/client-production": { - "version": "npm:@node-redis/client@1.0.0", - "resolved": "https://registry.npmjs.org/@node-redis/client/-/client-1.0.0.tgz", - "integrity": "sha512-DWDMeZELXG3rOGzCKfJEHCkCP6rgiA1H+oqj2N0NR4Q0fQUYMxTsyoqt80GpdYLilUW6zoCiQl9yL3vJhGhiCA==", - "requires": { - "cluster-key-slot": "1.1.0", - "generic-pool": "3.8.2", - "redis-parser": "3.0.0", + "ts-node": "^10.7.0", + "typedoc": "^0.22.13", + "typescript": "^4.6.3", "yallist": "4.0.0" } }, @@ -502,17 +459,17 @@ "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==" }, "debug": { - "version": "4.3.3", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.3.tgz", - "integrity": "sha512-/zxw5+vh1Tfv+4Qn7a5nsbcJKPaSvCDhojn6FEl9vupwK2VCSDtEiEtqr8DFtzYFOdz63LBkxec7DYuc2jon6Q==", + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", "requires": { "ms": "2.1.2" } }, "denque": { - "version": "1.5.1", - "resolved": "https://registry.npmjs.org/denque/-/denque-1.5.1.tgz", - "integrity": "sha512-XwE+iZ4D6ZUB7mfYRMb5wByE8L74HCn30FBN7sWnXksWc1LO1bPDl67pBR9o/kC4z/xSNAwkMYcGgqDV3BE3Hw==" + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/denque/-/denque-2.0.1.tgz", + "integrity": "sha512-tfiWc6BQLXNLpNiR5iGd0Ocu3P3VpxfzFiqubLgMfhfOw9WyvgJBd46CClNn9k3qfbjvT//0cf7AlYRX/OslMQ==" }, "emoji-regex": { "version": "8.0.0", @@ -524,39 +481,32 @@ "resolved": "https://registry.npmjs.org/escalade/-/escalade-3.1.1.tgz", "integrity": "sha512-k0er2gUkLf8O0zKJiAhmkTnJlTvINGv7ygDNPbeIsX/TJjGJZHuh9B2UxbsaEkmlEo9MfhrSzmhIlhRlI2GXnw==" }, - "generic-pool": { - "version": "3.8.2", - "resolved": "https://registry.npmjs.org/generic-pool/-/generic-pool-3.8.2.tgz", - "integrity": "sha512-nGToKy6p3PAbYQ7p1UlWl6vSPwfwU6TMSWK7TTu+WUY4ZjyZQGniGGt2oNVvyNSpyZYSB43zMXVLcBm08MTMkg==" - }, "get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", "integrity": "sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==" }, "hdr-histogram-js": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/hdr-histogram-js/-/hdr-histogram-js-2.0.1.tgz", - "integrity": "sha512-uPZxl1dAFnjUFHWLZmt93vUUvtHeaBay9nVNHu38SdOjMSF/4KqJUqa1Seuj08ptU1rEb6AHvB41X8n/zFZ74Q==", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/hdr-histogram-js/-/hdr-histogram-js-3.0.0.tgz", + "integrity": "sha512-/EpvQI2/Z98mNFYEnlqJ8Ogful8OpArLG/6Tf2bPnkutBVLIeMVNHjk1ZDfshF2BUweipzbk+dB1hgSB7SIakw==", "requires": { - "@assemblyscript/loader": "^0.10.1", + "@assemblyscript/loader": "^0.19.21", "base64-js": "^1.2.0", "pako": "^1.0.3" } }, "ioredis": { - "version": "4.28.1", - "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-4.28.1.tgz", - "integrity": "sha512-7gcrUJEcPHWy+eEyq6wIZpXtfHt8crhbc5+z0sqrnHUkwBblXinygfamj+/jx83Qo+2LW3q87Nj2VsuH6BF2BA==", + "version": "5.0.3", + "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-5.0.3.tgz", + "integrity": "sha512-hKxywVypoGGUJDyfnMufO0VNkuIaQufo2/PJ5SOYBKWgf4+gCnZEPaBPle615k08euY9/B9DKytVtwZROF7HaA==", "requires": { + "@ioredis/commands": "^1.1.1", "cluster-key-slot": "^1.1.0", - "debug": "^4.3.1", - "denque": "^1.1.0", + "debug": "^4.3.4", + "denque": "^2.0.1", "lodash.defaults": "^4.2.0", - "lodash.flatten": "^4.4.0", "lodash.isarguments": "^3.1.0", - "p-map": "^2.1.0", - "redis-commands": "1.7.0", "redis-errors": "^1.2.0", "redis-parser": "^3.0.0", "standard-as-callback": "^2.1.0" @@ -572,11 +522,6 @@ "resolved": "https://registry.npmjs.org/lodash.defaults/-/lodash.defaults-4.2.0.tgz", "integrity": "sha1-0JF4cW/+pN3p5ft7N/bwgCJ0WAw=" }, - "lodash.flatten": { - "version": "4.4.0", - "resolved": "https://registry.npmjs.org/lodash.flatten/-/lodash.flatten-4.4.0.tgz", - "integrity": "sha1-8xwiIlqWMtK7+OSt2+8kCqdlph8=" - }, "lodash.isarguments": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/lodash.isarguments/-/lodash.isarguments-3.1.0.tgz", @@ -587,11 +532,6 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, - "p-map": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/p-map/-/p-map-2.1.0.tgz", - "integrity": "sha512-y3b8Kpd8OAN444hxfBbFfj1FY/RjtTd8tzYwhUqNYXx0fXx2iX4maP4Qr6qhIKbQXI02wTLAda4fYUbDagTUFw==" - }, "pako": { "version": "1.0.11", "resolved": "https://registry.npmjs.org/pako/-/pako-1.0.11.tgz", @@ -624,6 +564,13 @@ "redis-commands": "^1.7.0", "redis-errors": "^1.2.0", "redis-parser": "^3.0.0" + }, + "dependencies": { + "denque": { + "version": "1.5.1", + "resolved": "https://registry.npmjs.org/denque/-/denque-1.5.1.tgz", + "integrity": "sha512-XwE+iZ4D6ZUB7mfYRMb5wByE8L74HCn30FBN7sWnXksWc1LO1bPDl67pBR9o/kC4z/xSNAwkMYcGgqDV3BE3Hw==" + } } }, "require-directory": { @@ -669,15 +616,10 @@ "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", "integrity": "sha512-0pfFzegeDWJHJIAmTLRP2DwHjdF5s7jo9tuztdQxAhINCdvS+3nGINqPd00AphqJR/0LhANUS6/+7SCb98YOfA==" }, - "yallist": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", - "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==" - }, "yargs": { - "version": "17.3.0", - "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.3.0.tgz", - "integrity": "sha512-GQl1pWyDoGptFPJx9b9L6kmR33TGusZvXIZUT+BOz9f7X2L94oeAskFYLEg/FkhV06zZPBYLvLZRWeYId29lew==", + "version": "17.4.0", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.4.0.tgz", + "integrity": "sha512-WJudfrk81yWFSOkZYpAZx4Nt7V4xp7S/uJkX0CnxovMCt1wCE8LNftPpNuF9X/u9gN5nsD7ycYtRcDf2pL3UiA==", "requires": { "cliui": "^7.0.2", "escalade": "^3.1.1", @@ -689,9 +631,9 @@ } }, "yargs-parser": { - "version": "21.0.0", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-21.0.0.tgz", - "integrity": "sha512-z9kApYUOCwoeZ78rfRYYWdiU/iNL6mwwYlkkZfJoyMR1xps+NEBX5X7XmRpxkZHhXJ6+Ey00IwKxBBSW9FIjyA==" + "version": "21.0.1", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-21.0.1.tgz", + "integrity": "sha512-9BK1jFpLzJROCI5TzwZL/TU4gqjK5xiHV/RfWLOahrjAko/e4DJkRDZQXfvqAsiZzzYhgAzbgz6lg48jcm4GLg==" } } } diff --git a/benchmark/package.json b/benchmark/package.json index 114b9e82d33..352451021c6 100644 --- a/benchmark/package.json +++ b/benchmark/package.json @@ -8,9 +8,9 @@ }, "dependencies": { "@node-redis/client": "../packages/client", - "hdr-histogram-js": "2.0.1", - "ioredis": "4.28.1", + "hdr-histogram-js": "3.0.0", + "ioredis": "5.0.3", "redis-v3": "npm:redis@3.1.2", - "yargs": "17.3.0" + "yargs": "17.4.0" } } From e141eb096c73e1579fd98d7274f32d744bf904a7 Mon Sep 17 00:00:00 2001 From: leibale Date: Tue, 5 Apr 2022 15:42:47 +0300 Subject: [PATCH 17/24] clean code --- packages/client/lib/client/RESP2/decoder.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index 26822f87e0c..f2c23fdccab 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -101,14 +101,14 @@ export default class RESP2Decoder { for (let i = this.cursor; i < chunk.length; i++) { if (chunk[i] === ASCII.CR) { const reply = composer.end( - chunk.slice(this.cursor, i) + chunk.subarray(this.cursor, i) ); this.cursor = i + 2; return reply; } } - const toWrite = chunk.slice(this.cursor); + const toWrite = chunk.subarray(this.cursor); composer.write(toWrite); this.cursor = chunk.length; } @@ -131,9 +131,7 @@ export default class RESP2Decoder { private parseInteger(chunk: Buffer): number | undefined { if (this.isNegativeInteger === undefined) { this.isNegativeInteger = chunk[this.cursor] === ASCII.MINUS; - if (this.isNegativeInteger) { - if (++this.cursor === chunk.length) return; - } + if (this.isNegativeInteger && ++this.cursor === chunk.length) return; } do { @@ -156,7 +154,7 @@ export default class RESP2Decoder { if (this.bulkStringRemainingLength === undefined) { const length = this.parseInteger(chunk); if (length === undefined) return; - else if (length === -1) return null; + if (length === -1) return null; this.bulkStringRemainingLength = length; @@ -166,14 +164,14 @@ export default class RESP2Decoder { const end = this.cursor + this.bulkStringRemainingLength; if (chunk.length >= end) { const reply = this.currentStringComposer.end( - chunk.slice(this.cursor, end) + chunk.subarray(this.cursor, end) ); this.bulkStringRemainingLength = undefined; this.cursor = end + 2; return reply; } - const toWrite = chunk.slice(this.cursor); + const toWrite = chunk.subarray(this.cursor); this.currentStringComposer.write(toWrite); this.bulkStringRemainingLength -= toWrite.length; this.cursor = chunk.length; From aaaa15cf00f95fc743065da5d901de450f88e6e9 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 20:23:42 -0400 Subject: [PATCH 18/24] fix socket error handlers, reset parser on error --- packages/client/lib/client/commands-queue.ts | 1 + packages/client/lib/client/socket.ts | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 8af200314b8..47f5859edbf 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -390,6 +390,7 @@ export default class RedisCommandsQueue { } flushWaitingForReply(err: Error): void { + this.#parser.reset(); RedisCommandsQueue.#flushQueue(this.#waitingForReply, err); if (!this.#chainInExecution) return; diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 0366b2b86e1..1b1353b3c91 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -116,14 +116,13 @@ export default class RedisSocket extends EventEmitter { if (this.#initiator) { try { await this.#initiator(); - } catch (err) { - this.#socket.destroy(); - this.#socket = undefined; - + } catch (err: any) { if (err instanceof AuthError) { this.#isOpen = false; } + this.#socket.destroy(err); + throw err; } From cfbfa2ed1a8f814c7a11efe41710ce6bf1cb373e Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 20:26:14 -0400 Subject: [PATCH 19/24] fix #2080 - reset pubSubState on socket error --- packages/client/lib/client/commands-queue.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 47f5859edbf..08d4ced7209 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -391,6 +391,7 @@ export default class RedisCommandsQueue { flushWaitingForReply(err: Error): void { this.#parser.reset(); + this.#pubSubState = undefined; RedisCommandsQueue.#flushQueue(this.#waitingForReply, err); if (!this.#chainInExecution) return; From ce3728290b85d86c8103f3f971f200c26a06e04b Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 20:38:33 -0400 Subject: [PATCH 20/24] reset decoder on socket error --- packages/client/lib/client/RESP2/composers/buffer.ts | 4 ++++ packages/client/lib/client/RESP2/composers/interface.ts | 2 ++ packages/client/lib/client/RESP2/composers/string.ts | 4 ++++ packages/client/lib/client/RESP2/decoder.ts | 8 ++++++++ packages/client/lib/client/commands-queue.ts | 2 +- 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/client/lib/client/RESP2/composers/buffer.ts b/packages/client/lib/client/RESP2/composers/buffer.ts index eefec0aec89..4affb4283e0 100644 --- a/packages/client/lib/client/RESP2/composers/buffer.ts +++ b/packages/client/lib/client/RESP2/composers/buffer.ts @@ -11,4 +11,8 @@ export default class BufferComposer implements Composer { this.write(buffer); return Buffer.concat(this.chunks.splice(0)); } + + reset() { + this.chunks = []; + } } diff --git a/packages/client/lib/client/RESP2/composers/interface.ts b/packages/client/lib/client/RESP2/composers/interface.ts index 6e7106c9cf9..0fc8f031414 100644 --- a/packages/client/lib/client/RESP2/composers/interface.ts +++ b/packages/client/lib/client/RESP2/composers/interface.ts @@ -2,4 +2,6 @@ export interface Composer { write(buffer: Buffer): void; end(buffer: Buffer): T; + + reset(): void; } diff --git a/packages/client/lib/client/RESP2/composers/string.ts b/packages/client/lib/client/RESP2/composers/string.ts index 8684538b45e..0cd8f00e95c 100644 --- a/packages/client/lib/client/RESP2/composers/string.ts +++ b/packages/client/lib/client/RESP2/composers/string.ts @@ -15,4 +15,8 @@ export default class StringComposer implements Composer { this.string = ''; return string; } + + reset() { + this.string = ''; + } } diff --git a/packages/client/lib/client/RESP2/decoder.ts b/packages/client/lib/client/RESP2/decoder.ts index f2c23fdccab..e9d2317deab 100644 --- a/packages/client/lib/client/RESP2/decoder.ts +++ b/packages/client/lib/client/RESP2/decoder.ts @@ -51,6 +51,14 @@ export default class RESP2Decoder { private currentStringComposer: BufferComposer | StringComposer = this.stringComposer; + reset() { + this.cursor = 0; + this.type = undefined; + this.bufferComposer.reset(); + this.stringComposer.reset(); + this.currentStringComposer = this.stringComposer; + } + write(chunk: Buffer): void { while (this.cursor < chunk.length) { if (!this.type) { diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 761e630cf3d..22a12a69292 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -397,7 +397,7 @@ export default class RedisCommandsQueue { } flushWaitingForReply(err: Error): void { - this.#parser.reset(); + this.#decoder.reset(); this.#pubSubState = undefined; RedisCommandsQueue.#flushQueue(this.#waitingForReply, err); From 8ad0473a1c796e149913b52dfb2da4887e85bf02 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 22:14:11 -0400 Subject: [PATCH 21/24] fix pubsub --- packages/client/lib/client/commands-queue.ts | 88 ++++++++------------ packages/client/lib/client/index.spec.ts | 33 +------- packages/client/lib/client/index.ts | 6 +- packages/client/lib/client/socket.ts | 69 +++++---------- packages/client/lib/errors.ts | 6 -- 5 files changed, 62 insertions(+), 140 deletions(-) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 08d4ced7209..addc29e5afe 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -53,16 +53,6 @@ interface PubSubListeners { type PubSubListenersMap = Map; -interface PubSubState { - subscribing: number; - subscribed: number; - unsubscribing: number; - listeners: { - channels: PubSubListenersMap; - patterns: PubSubListenersMap; - }; -} - export default class RedisCommandsQueue { static #flushQueue(queue: LinkedList, err: Error): void { while (queue.length) { @@ -98,7 +88,16 @@ export default class RedisCommandsQueue { readonly #waitingForReply = new LinkedList(); - #pubSubState: PubSubState | undefined; + readonly #pubSubState = { + isActive: false, + subscribing: 0, + subscribed: 0, + unsubscribing: 0, + listeners: { + channels: new Map(), + patterns: new Map() + } + }; static readonly #PUB_SUB_MESSAGES = { message: Buffer.from('message'), @@ -111,7 +110,7 @@ export default class RedisCommandsQueue { readonly #parser = new RedisParser({ returnReply: (reply: unknown) => { - if (this.#pubSubState && Array.isArray(reply)) { + if (this.#pubSubState.isActive && Array.isArray(reply)) { if (RedisCommandsQueue.#PUB_SUB_MESSAGES.message.equals(reply[0])) { return RedisCommandsQueue.#emitPubSubMessage( this.#pubSubState.listeners.channels, @@ -150,7 +149,7 @@ export default class RedisCommandsQueue { } addCommand(args: RedisCommandArguments, options?: QueueCommandOptions): Promise { - if (this.#pubSubState && !options?.ignorePubSubMode) { + if (this.#pubSubState.isActive && !options?.ignorePubSubMode) { return Promise.reject(new Error('Cannot send commands in PubSub mode')); } else if (this.#maxLength && this.#waitingToBeSent.length + this.#waitingForReply.length >= this.#maxLength) { return Promise.reject(new Error('The queue is full')); @@ -190,27 +189,16 @@ export default class RedisCommandsQueue { }); } - #initiatePubSubState(): PubSubState { - return this.#pubSubState ??= { - subscribed: 0, - subscribing: 0, - unsubscribing: 0, - listeners: { - channels: new Map(), - patterns: new Map() - } - }; - } - subscribe( command: PubSubSubscribeCommands, channels: RedisCommandArgument | Array, listener: PubSubListener, returnBuffers?: T ): Promise { - const pubSubState = this.#initiatePubSubState(), - channelsToSubscribe: Array = [], - listenersMap = command === PubSubSubscribeCommands.SUBSCRIBE ? pubSubState.listeners.channels : pubSubState.listeners.patterns; + const channelsToSubscribe: Array = [], + listenersMap = command === PubSubSubscribeCommands.SUBSCRIBE ? + this.#pubSubState.listeners.channels : + this.#pubSubState.listeners.patterns; for (const channel of (Array.isArray(channels) ? channels : [channels])) { const channelString = typeof channel === 'string' ? channel : channel.toString(); let listeners = listenersMap.get(channelString); @@ -230,6 +218,7 @@ export default class RedisCommandsQueue { if (!channelsToSubscribe.length) { return Promise.resolve(); } + return this.#pushPubSubCommand(command, channelsToSubscribe); } @@ -239,10 +228,6 @@ export default class RedisCommandsQueue { listener?: PubSubListener, returnBuffers?: T ): Promise { - if (!this.#pubSubState) { - return Promise.resolve(); - } - const listeners = command === PubSubUnsubscribeCommands.UNSUBSCRIBE ? this.#pubSubState.listeners.channels : this.#pubSubState.listeners.patterns; @@ -280,8 +265,7 @@ export default class RedisCommandsQueue { #pushPubSubCommand(command: PubSubSubscribeCommands | PubSubUnsubscribeCommands, channels: number | Array): Promise { return new Promise((resolve, reject) => { - const pubSubState = this.#initiatePubSubState(), - isSubscribe = command === PubSubSubscribeCommands.SUBSCRIBE || command === PubSubSubscribeCommands.PSUBSCRIBE, + const isSubscribe = command === PubSubSubscribeCommands.SUBSCRIBE || command === PubSubSubscribeCommands.PSUBSCRIBE, inProgressKey = isSubscribe ? 'subscribing' : 'unsubscribing', commandArgs: Array = [command]; @@ -293,38 +277,42 @@ export default class RedisCommandsQueue { channelsCounter = channels.length; } - pubSubState[inProgressKey] += channelsCounter; + this.#pubSubState.isActive = true; + this.#pubSubState[inProgressKey] += channelsCounter; this.#waitingToBeSent.push({ args: commandArgs, channelsCounter, returnBuffers: true, resolve: () => { - pubSubState[inProgressKey] -= channelsCounter; - if (isSubscribe) { - pubSubState.subscribed += channelsCounter; - } else { - pubSubState.subscribed -= channelsCounter; - if (!pubSubState.subscribed && !pubSubState.subscribing && !pubSubState.subscribed) { - this.#pubSubState = undefined; - } - } + this.#pubSubState[inProgressKey] -= channelsCounter; + this.#pubSubState.subscribed += channelsCounter * (isSubscribe ? 1 : -1); + this.#updatePubSubActiveState(); resolve(); }, reject: err => { - pubSubState[inProgressKey] -= channelsCounter * (isSubscribe ? 1 : -1); + this.#pubSubState[inProgressKey] -= channelsCounter * (isSubscribe ? 1 : -1); + this.#updatePubSubActiveState(); reject(err); } }); }); } - resubscribe(): Promise | undefined { - if (!this.#pubSubState) { - return; + #updatePubSubActiveState(): void { + if ( + !this.#pubSubState.subscribed && + !this.#pubSubState.subscribing && + !this.#pubSubState.subscribed + ) { + this.#pubSubState.isActive = false; } + } + resubscribe(): Promise | undefined { this.#pubSubState.subscribed = 0; + this.#pubSubState.subscribing = 0; + this.#pubSubState.unsubscribing = 0; const promises = [], { channels, patterns } = this.#pubSubState.listeners; @@ -369,8 +357,7 @@ export default class RedisCommandsQueue { #setReturnBuffers() { this.#parser.setReturnBuffers( !!this.#waitingForReply.head?.value.returnBuffers || - !!this.#pubSubState?.subscribed || - !!this.#pubSubState?.subscribing + !!this.#pubSubState.isActive ); } @@ -391,7 +378,6 @@ export default class RedisCommandsQueue { flushWaitingForReply(err: Error): void { this.#parser.reset(); - this.#pubSubState = undefined; RedisCommandsQueue.#flushQueue(this.#waitingForReply, err); if (!this.#chainInExecution) return; diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index c48505c7586..09b974c910b 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -3,7 +3,7 @@ import testUtils, { GLOBAL, waitTillBeenCalled } from '../test-utils'; import RedisClient, { RedisClientType } from '.'; import { RedisClientMultiCommandType } from './multi-command'; import { RedisCommandArguments, RedisCommandRawReply, RedisModules, RedisScripts } from '../commands'; -import { AbortError, AuthError, ClientClosedError, ConnectionTimeoutError, DisconnectsClientError, SocketClosedUnexpectedlyError, WatchError } from '../errors'; +import { AbortError, ClientClosedError, ConnectionTimeoutError, DisconnectsClientError, SocketClosedUnexpectedlyError, WatchError } from '../errors'; import { defineScript } from '../lua-script'; import { spy } from 'sinon'; import { once } from 'events'; @@ -87,30 +87,6 @@ describe('Client', () => { ); }, GLOBAL.SERVERS.PASSWORD); - testUtils.testWithClient('should not retry connecting if failed due to wrong auth', async client => { - let message; - if (testUtils.isVersionGreaterThan([6, 2])) { - message = 'WRONGPASS invalid username-password pair or user is disabled.'; - } else if (testUtils.isVersionGreaterThan([6])) { - message = 'WRONGPASS invalid username-password pair'; - } else { - message = 'ERR invalid password'; - } - - await assert.rejects( - client.connect(), - new AuthError(message) - ); - - assert.equal(client.isOpen, false); - }, { - ...GLOBAL.SERVERS.PASSWORD, - clientOptions: { - password: 'wrongpassword' - }, - disableClientSetup: true - }); - testUtils.testWithClient('should execute AUTH before SELECT', async client => { assert.equal( (await client.clientInfo()).db, @@ -300,7 +276,8 @@ describe('Client', () => { await client.multi() .sAdd('a', ['b', 'c']) .v4.exec(), - [2]) + [2] + ); }, { ...GLOBAL.SERVERS.OPEN, clientOptions: { @@ -681,10 +658,6 @@ describe('Client', () => { const listener = spy(); await subscriber.subscribe('channel', listener); - subscriber.on('error', err => { - console.error('subscriber err', err.message); - }); - await Promise.all([ once(subscriber, 'error'), publisher.clientKill({ diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 1dd74fa1afe..25535e0728e 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -11,7 +11,7 @@ import { ScanCommandOptions } from '../commands/SCAN'; import { HScanTuple } from '../commands/HSCAN'; import { extendWithCommands, extendWithModulesAndScripts, transformCommandArguments, transformCommandReply } from '../commander'; import { Pool, Options as PoolOptions, createPool } from 'generic-pool'; -import { ClientClosedError, DisconnectsClientError, AuthError } from '../errors'; +import { ClientClosedError, DisconnectsClientError } from '../errors'; import { URL } from 'url'; import { TcpSocketConnectOpts } from 'net'; @@ -254,9 +254,7 @@ export default class RedisClient password: this.#options.password ?? '' }), { asap: true } - ).catch(err => { - throw new AuthError(err.message); - }) + ) ); } diff --git a/packages/client/lib/client/socket.ts b/packages/client/lib/client/socket.ts index 1b1353b3c91..b04950a0724 100644 --- a/packages/client/lib/client/socket.ts +++ b/packages/client/lib/client/socket.ts @@ -3,7 +3,7 @@ import * as net from 'net'; import * as tls from 'tls'; import { encodeCommand } from '../commander'; import { RedisCommandArguments } from '../commands'; -import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, AuthError, ReconnectStrategyError } from '../errors'; +import { ConnectionTimeoutError, ClientClosedError, SocketClosedUnexpectedlyError, ReconnectStrategyError } from '../errors'; import { promiseTimeout } from '../utils'; export interface RedisSocketCommonOptions { @@ -53,7 +53,7 @@ export default class RedisSocket extends EventEmitter { return (options as RedisTlsSocketOptions).tls === true; } - readonly #initiator?: RedisSocketInitiator; + readonly #initiator: RedisSocketInitiator; readonly #options: RedisSocketOptions; @@ -79,7 +79,7 @@ export default class RedisSocket extends EventEmitter { return this.#writableNeedDrain; } - constructor(initiator?: RedisSocketInitiator, options?: RedisSocketOptions) { + constructor(initiator: RedisSocketInitiator, options?: RedisSocketOptions) { super(); this.#initiator = initiator; @@ -91,69 +91,40 @@ export default class RedisSocket extends EventEmitter { throw new Error('Socket already opened'); } - return this.#connect(); + return this.#connect(0); } - async #connect(hadError?: boolean): Promise { + async #connect(retries: number, hadError?: boolean): Promise { + if (retries > 0 || hadError) { + this.emit('reconnecting'); + } + try { this.#isOpen = true; - this.#socket = await this.#retryConnection(0, hadError); + this.#socket = await this.#createSocket(); this.#writableNeedDrain = false; - } catch (err) { - this.#isOpen = false; - this.emit('error', err); - this.emit('end'); - throw err; - } + this.emit('connect'); - if (!this.#isOpen) { - this.disconnect(); - return; - } - - this.emit('connect'); - - if (this.#initiator) { try { await this.#initiator(); - } catch (err: any) { - if (err instanceof AuthError) { - this.#isOpen = false; - } - - this.#socket.destroy(err); - + } catch (err) { + this.#socket.destroy(); + this.#socket = undefined; throw err; } - - if (!this.#isOpen) return; - } - - this.#isReady = true; - - this.emit('ready'); - } - - async #retryConnection(retries: number, hadError?: boolean): Promise { - if (retries > 0 || hadError) { - this.emit('reconnecting'); - } - - try { - return await this.#createSocket(); + this.#isReady = true; + this.emit('ready'); } catch (err) { - if (!this.#isOpen) { - throw err; - } + this.emit('error', err); const retryIn = (this.#options?.reconnectStrategy ?? RedisSocket.#defaultReconnectStrategy)(retries); if (retryIn instanceof Error) { + this.#isOpen = false; throw new ReconnectStrategyError(retryIn, err); } - this.emit('error', err); await promiseTimeout(retryIn); - return this.#retryConnection(retries + 1); + return this.#connect(retries + 1); } } @@ -211,7 +182,7 @@ export default class RedisSocket extends EventEmitter { this.#isReady = false; this.emit('error', err); - this.#connect(true).catch(() => { + this.#connect(0, true).catch(() => { // the error was already emitted, silently ignore it }); } diff --git a/packages/client/lib/errors.ts b/packages/client/lib/errors.ts index e43dbc81422..01dff992290 100644 --- a/packages/client/lib/errors.ts +++ b/packages/client/lib/errors.ts @@ -34,12 +34,6 @@ export class SocketClosedUnexpectedlyError extends Error { } } -export class AuthError extends Error { - constructor(message: string) { - super(message); - } -} - export class RootNodesUnavailableError extends Error { constructor() { super('All the root nodes are unavailable'); From 2c237877c4194a0b198df482414167c4b451b420 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 22:21:07 -0400 Subject: [PATCH 22/24] fix "RedisSocketInitiator" --- packages/client/lib/client/socket.spec.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/client/lib/client/socket.spec.ts b/packages/client/lib/client/socket.spec.ts index 4c5cfd1d9b3..54f84eb9fe0 100644 --- a/packages/client/lib/client/socket.spec.ts +++ b/packages/client/lib/client/socket.spec.ts @@ -21,10 +21,13 @@ describe('Socket', () => { return time; }); - const socket = new RedisSocket(undefined, { - host: 'error', - reconnectStrategy - }); + const socket = new RedisSocket( + () => Promise.resolve(), + { + host: 'error', + reconnectStrategy + } + ); socket.on('error', () => { // ignore errors From e307d1fbd17f945a9d7d72521f9c8d69ea17b1fe Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 22:39:11 -0400 Subject: [PATCH 23/24] fix returnStringsAsBuffers --- packages/client/lib/client/commands-queue.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index c2113e6cc98..4714679ce37 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -108,7 +108,8 @@ export default class RedisCommandsQueue { #decoder = new RESP2Decoder({ returnStringsAsBuffers: () => { - return !!this.#waitingForReply.head?.value.returnBuffers || !!this.#pubSubState; + return !!this.#waitingForReply.head?.value.returnBuffers || + this.#pubSubState.isActive; }, onReply: reply => { if (this.#handlePubSubReply(reply)) { From 8ec485514ec27d7fc6bf2fce842b485932b1f1a7 Mon Sep 17 00:00:00 2001 From: leibale Date: Wed, 20 Apr 2022 22:43:06 -0400 Subject: [PATCH 24/24] fix merge --- packages/client/lib/client/commands-queue.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client/lib/client/commands-queue.ts b/packages/client/lib/client/commands-queue.ts index 4714679ce37..8cae914963e 100644 --- a/packages/client/lib/client/commands-queue.ts +++ b/packages/client/lib/client/commands-queue.ts @@ -356,7 +356,7 @@ export default class RedisCommandsQueue { } #handlePubSubReply(reply: any): boolean { - if (!this.#pubSubState || !Array.isArray(reply)) return false; + if (!this.#pubSubState.isActive || !Array.isArray(reply)) return false; if (RedisCommandsQueue.#PUB_SUB_MESSAGES.message.equals(reply[0])) { RedisCommandsQueue.#emitPubSubMessage(