From f4680f0849a0516453bcab1a23785cbb2888f287 Mon Sep 17 00:00:00 2001 From: Leibale Eidelman Date: Mon, 18 Dec 2023 15:15:21 -0500 Subject: [PATCH] fix #2665 - handle errors in multi/pipeline replies (#2666) * fix #2665 - handle errors in multi/pipeline replies * fix MultiErrorReply replies type * run tests on all versions, remove console.log, fix bug * add errors iterator helper * test `.errors()` as well --- packages/client/lib/client/index.spec.ts | 19 ++++++++++++++++++- packages/client/lib/errors.ts | 19 +++++++++++++++++++ packages/client/lib/multi-command.ts | 22 +++++++++++++++------- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 3278d27775..4442d3adb8 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 { RedisCommandRawReply, RedisModules, RedisFunctions, RedisScripts } from '../commands'; -import { AbortError, ClientClosedError, ClientOfflineError, ConnectionTimeoutError, DisconnectsClientError, SocketClosedUnexpectedlyError, WatchError } from '../errors'; +import { AbortError, ClientClosedError, ClientOfflineError, ConnectionTimeoutError, DisconnectsClientError, ErrorReply, MultiErrorReply, SocketClosedUnexpectedlyError, WatchError } from '../errors'; import { defineScript } from '../lua-script'; import { spy } from 'sinon'; import { once } from 'events'; @@ -602,6 +602,23 @@ describe('Client', () => { ...GLOBAL.SERVERS.OPEN, minimumDockerVersion: [6, 2] // CLIENT INFO }); + + testUtils.testWithClient('should handle error replies (#2665)', async client => { + await assert.rejects( + client.multi() + .set('key', 'value') + .hGetAll('key') + .exec(), + err => { + assert.ok(err instanceof MultiErrorReply); + assert.equal(err.replies.length, 2); + assert.deepEqual(err.errorIndexes, [1]); + assert.ok(err.replies[1] instanceof ErrorReply); + assert.deepEqual([...err.errors()], [err.replies[1]]); + return true; + } + ); + }, GLOBAL.SERVERS.OPEN); }); testUtils.testWithClient('scripts', async client => { diff --git a/packages/client/lib/errors.ts b/packages/client/lib/errors.ts index 3070970315..aa97d9cf26 100644 --- a/packages/client/lib/errors.ts +++ b/packages/client/lib/errors.ts @@ -1,3 +1,5 @@ +import { RedisCommandRawReply } from './commands'; + export class AbortError extends Error { constructor() { super('The command was aborted'); @@ -63,3 +65,20 @@ export class ErrorReply extends Error { this.stack = undefined; } } + +export class MultiErrorReply extends ErrorReply { + replies; + errorIndexes; + + constructor(replies: Array, errorIndexes: Array) { + super(`${errorIndexes.length} commands failed, see .replies and .errorIndexes for more information`); + this.replies = replies; + this.errorIndexes = errorIndexes; + } + + *errors() { + for (const index of this.errorIndexes) { + yield this.replies[index]; + } + } +} diff --git a/packages/client/lib/multi-command.ts b/packages/client/lib/multi-command.ts index 08f23ffa45..642c2ea36c 100644 --- a/packages/client/lib/multi-command.ts +++ b/packages/client/lib/multi-command.ts @@ -1,6 +1,6 @@ import { fCallArguments } from './commander'; import { RedisCommand, RedisCommandArguments, RedisCommandRawReply, RedisFunction, RedisScript } from './commands'; -import { WatchError } from './errors'; +import { ErrorReply, MultiErrorReply, WatchError } from './errors'; export interface RedisMultiQueuedCommand { args: RedisCommandArguments; @@ -69,7 +69,7 @@ export default class RedisMultiCommand { return transformedArguments; } - handleExecReplies(rawReplies: Array): Array { + handleExecReplies(rawReplies: Array): Array { const execReply = rawReplies[rawReplies.length - 1] as (null | Array); if (execReply === null) { throw new WatchError(); @@ -78,10 +78,18 @@ export default class RedisMultiCommand { return this.transformReplies(execReply); } - transformReplies(rawReplies: Array): Array { - return rawReplies.map((reply, i) => { - const { transformReply, args } = this.queue[i]; - return transformReply ? transformReply(reply, args.preserve) : reply; - }); + transformReplies(rawReplies: Array): Array { + const errorIndexes: Array = [], + replies = rawReplies.map((reply, i) => { + if (reply instanceof ErrorReply) { + errorIndexes.push(i); + return reply; + } + const { transformReply, args } = this.queue[i]; + return transformReply ? transformReply(reply, args.preserve) : reply; + }); + + if (errorIndexes.length) throw new MultiErrorReply(replies, errorIndexes); + return replies; } }