Skip to content

Commit

Permalink
feat: skip ready check on NOPERM error
Browse files Browse the repository at this point in the history
Closes #1293

BREAKING CHANGE: `Redis#serverInfo` is removed. This field is never documented so
you very likely have never used it.
  • Loading branch information
luin committed Feb 28, 2022
1 parent e1cd0cf commit 5b7a87d
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
},
"overrides": [
{
"files": ["test/**/*"],
"files": ["test/**/*", "test-cluster/**/*"],
"env": {
"mocha": true
},
Expand Down
8 changes: 7 additions & 1 deletion lib/Redis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,12 @@ class Redis extends Commander {
const _this = this;
this.info(function (err, res) {
if (err) {
if (err.message && err.message.includes("NOPERM")) {
console.warn(
`Skipping the ready check because INFO command fails: "${err.message}". You can disable ready check with "enableReadyCheck". More: https://github.com/luin/ioredis/wiki/Disable-ready-check.`
);
return callback(null, {});
}
return callback(err);
}
if (typeof res !== "string") {
Expand Down Expand Up @@ -799,7 +805,7 @@ class Redis extends Commander {
_this._readyCheck(callback);
}, retryTime);
}
});
}).catch(noop);
}
}

Expand Down
1 change: 0 additions & 1 deletion lib/redis/event_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export function connectHandler(self) {
);
}
} else {
self.serverInfo = info;
if (self.connector.check(info)) {
exports.readyHandler(self)();
} else {
Expand Down
6 changes: 6 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"standard-as-callback": "^2.1.0"
},
"devDependencies": {
"@types/chai": "^4.3.0",
"@types/debug": "^4.1.5",
"@types/lodash.defaults": "^4.2.6",
"@types/lodash.isarguments": "^3.1.6",
Expand Down
2 changes: 1 addition & 1 deletion test/functional/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe("connection", function () {
redis.disconnect();
setImmediate(() => done());
}
command.resolve("fake");
return command.resolve("fake");
});
});

Expand Down
4 changes: 2 additions & 2 deletions test/functional/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ describe("monitor", () => {
it("should wait for the ready event before monitoring", (done) => {
const redis = new Redis();
redis.on("ready", () => {
// @ts-expect-error
const readyCheck = sinon.spy(Redis.prototype, "_readyCheck");
redis.monitor(function (err, monitor) {
redis.monitor((err, monitor) => {
expect(readyCheck.callCount).to.eql(1);
Redis.prototype._readyCheck.restore();
redis.disconnect();
monitor.disconnect();
done();
Expand Down
2 changes: 1 addition & 1 deletion test/functional/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe("pipeline", () => {
const redis = new Redis({ keyPrefix: "foo:" });
redis.addBuiltinCommand("someCommand");
sinon.stub(redis, "sendCommand").callsFake((command) => {
command.resolve(Buffer.from("OK"));
return command.resolve(Buffer.from("OK"));
});
const result = await redis.pipeline().someCommand().exec();
expect(result).to.eql([[null, "OK"]]);
Expand Down
43 changes: 34 additions & 9 deletions test/functional/ready_check.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
import Redis from "../../lib/Redis";
import { noop } from "../../lib/utils";
import * as sinon from "sinon";
import { expect } from "chai";

const stubInfo = (
redis: Redis,
response: [Error | null, string | undefined]
) => {
sinon.stub(redis, "info").callsFake((section, callback) => {
const cb = typeof section === "function" ? section : callback;
const [error, info] = response;
cb(error, info);
return error ? Promise.reject(error) : Promise.resolve(info);
});
};

describe("ready_check", () => {
it("should retry when redis is not ready", (done) => {
const redis = new Redis({ lazyConnect: true });

sinon.stub(redis, "info").callsFake((callback) => {
callback(null, "loading:1\r\nloading_eta_seconds:7");
});
stubInfo(redis, [null, "loading:1\r\nloading_eta_seconds:7"]);

// @ts-expect-error
const stub = sinon.stub(global, "setTimeout").callsFake((body, ms) => {
sinon.stub(global, "setTimeout").callsFake((_body, ms) => {
if (ms === 7000) {
redis.info.restore();
stub.restore();
done();
}
});
Expand All @@ -29,10 +39,25 @@ describe("ready_check", () => {
},
});

sinon.stub(redis, "info").callsFake((callback) => {
callback(new Error("info error"));
});
stubInfo(redis, [new Error("info error"), undefined]);

redis.connect().catch(noop);
});

it("warns for NOPERM error", async () => {
const redis = new Redis({
lazyConnect: true,
});

const warn = sinon.stub(console, "warn");
stubInfo(redis, [
new Error(
"NOPERM this user has no permissions to run the 'info' command"
),
undefined,
]);

await redis.connect();
expect(warn.calledOnce).to.eql(true);
});
});

0 comments on commit 5b7a87d

Please sign in to comment.