fix: Fix the NOSCRIPT behavior when using pipelines

redis · Jan 31, 2022 · f36a582 · f36a582
1 parent bdf88bd
commit f36a582
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 31 deletions.
diff --git 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
@@ -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();
     });