From f36a582bb1399b7d680d94739fcd693814123daf Mon Sep 17 00:00:00 2001 From: Marc Bachmann Date: Mon, 31 Jan 2022 13:12:36 +0100 Subject: [PATCH] fix: Fix the NOSCRIPT behavior when using pipelines --- lib/script.ts | 32 +++++++++++++------------------- test/functional/pipeline.ts | 29 +++++++++++++++++------------ 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/lib/script.ts b/lib/script.ts index de09466e8..976c0cd7e 100644 --- a/lib/script.ts +++ b/lib/script.ts @@ -1,5 +1,4 @@ import { createHash } from "crypto"; -import { isPromise } from "./promiseContainer"; import Command from "./command"; import asCallback from "standard-as-callback"; import { CallbackFunction } from "./types"; @@ -34,26 +33,21 @@ export default class Script { const evalsha = new Command("evalsha", [this.sha].concat(args), options); evalsha.isCustomCommand = true; + evalsha.promise = evalsha.promise.catch((err: Error) => { + if (err.toString().indexOf("NOSCRIPT") === -1) { + throw err; + } + const command = new Command("eval", [this.lua].concat(args), options); + if (container.isPipeline === true) container.redis.sendCommand(command); + else container.sendCommand(command); + return command.promise; + }); - const result = container.sendCommand(evalsha); - if (isPromise(result)) { - return asCallback( - result.catch((err: Error) => { - if (err.toString().indexOf("NOSCRIPT") === -1) { - throw err; - } - return container.sendCommand( - new Command("eval", [this.lua].concat(args), options) - ); - }), - callback - ); - } - - // result is not a Promise--probably returned from a pipeline chain; however, - // we still need the callback to fire when the script is evaluated asCallback(evalsha.promise, callback); - return result; + // The result here is one of + // - a Promise when executed on the redis instance + // - a pipeline instance in pipeline mode + return container.sendCommand(evalsha); } } diff --git a/test/functional/pipeline.ts b/test/functional/pipeline.ts index cd0228cbe..f35b66313 100644 --- a/test/functional/pipeline.ts +++ b/test/functional/pipeline.ts @@ -357,14 +357,16 @@ describe("pipeline", function () { it("should reload scripts on redis restart (reconnect)", async function () { const redis = new Redis({ connectionName: "load-script-on-reconnect" }); const redis2 = new Redis(); - redis.defineCommand("exeecafterreconnect", { + redis.defineCommand("execafterreconnect", { numberOfKeys: 0, - lua: `return "OK"`, + lua: `return "Foo"`, }); - const [[err, res]] = await redis.multi([["exeecafterreconnect"]]).exec(); + const [[err, res]] = await redis + .pipeline([["execafterreconnect"]]) + .exec(); expect(err).to.equal(null); - expect(res).to.equal("OK"); + expect(res).to.equal("Foo"); const client = await redis.client("list").then((clients) => { const myInfo = clients @@ -378,16 +380,19 @@ describe("pipeline", function () { await redis2.script("flush"); await redis2.client("kill", "addr", client); - // Wait for reconnect, at the moment scripts are not loaded - // if the pipeline starts before ioredis reconnects - await redis.ping(); - - const [[err2, res2]] = await redis - .multi([["exeecafterreconnect"]]) + const res2 = await redis + .pipeline([ + ["set", "foo", "bar"], + ["execafterreconnect"], + ["get", "foo"], + ]) .exec(); - expect(err2).to.equal(null); - expect(res2).to.equal("OK"); + expect(res2).to.deep.equal([ + [null, "OK"], + [null, "Foo"], + [null, "bar"], + ]); redis.disconnect(); redis2.disconnect(); });