Skip to content

Commit b530a0b

Browse files
committedMar 14, 2022
feat: skip ready check on NOPERM error
Closes #1293 BREAKING CHANGE: `Redis#serverInfo` is removed. This field is never documented so you very likely have never used it.
1 parent 1eba58b commit b530a0b

9 files changed

+53
-16
lines changed
 

‎.eslintrc.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
},
6161
"overrides": [
6262
{
63-
"files": ["test/**/*"],
63+
"files": ["test/**/*", "test-cluster/**/*"],
6464
"env": {
6565
"mocha": true
6666
},

‎lib/Redis.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,12 @@ class Redis extends Commander {
766766
const _this = this;
767767
this.info(function (err, res) {
768768
if (err) {
769+
if (err.message && err.message.includes("NOPERM")) {
770+
console.warn(
771+
`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.`
772+
);
773+
return callback(null, {});
774+
}
769775
return callback(err);
770776
}
771777
if (typeof res !== "string") {
@@ -799,7 +805,7 @@ class Redis extends Commander {
799805
_this._readyCheck(callback);
800806
}, retryTime);
801807
}
802-
});
808+
}).catch(noop);
803809
}
804810
}
805811

‎lib/redis/event_handler.ts

-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ export function connectHandler(self) {
8888
);
8989
}
9090
} else {
91-
self.serverInfo = info;
9291
if (self.connector.check(info)) {
9392
exports.readyHandler(self)();
9493
} else {

‎package-lock.json

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"standard-as-callback": "^2.1.0"
5050
},
5151
"devDependencies": {
52+
"@types/chai": "^4.3.0",
5253
"@types/debug": "^4.1.5",
5354
"@types/lodash.defaults": "^4.2.6",
5455
"@types/lodash.isarguments": "^3.1.6",

‎test/functional/connection.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe("connection", function () {
3737
redis.disconnect();
3838
setImmediate(() => done());
3939
}
40-
command.resolve("fake");
40+
return command.resolve("fake");
4141
});
4242
});
4343

‎test/functional/monitor.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,10 @@ describe("monitor", () => {
6464
it("should wait for the ready event before monitoring", (done) => {
6565
const redis = new Redis();
6666
redis.on("ready", () => {
67+
// @ts-expect-error
6768
const readyCheck = sinon.spy(Redis.prototype, "_readyCheck");
68-
redis.monitor(function (err, monitor) {
69+
redis.monitor((err, monitor) => {
6970
expect(readyCheck.callCount).to.eql(1);
70-
Redis.prototype._readyCheck.restore();
7171
redis.disconnect();
7272
monitor.disconnect();
7373
done();

‎test/functional/pipeline.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ describe("pipeline", () => {
144144
const redis = new Redis({ keyPrefix: "foo:" });
145145
redis.addBuiltinCommand("someCommand");
146146
sinon.stub(redis, "sendCommand").callsFake((command) => {
147-
command.resolve(Buffer.from("OK"));
147+
return command.resolve(Buffer.from("OK"));
148148
});
149149
const result = await redis.pipeline().someCommand().exec();
150150
expect(result).to.eql([[null, "OK"]]);

‎test/functional/ready_check.ts

+34-9
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,29 @@
11
import Redis from "../../lib/Redis";
22
import { noop } from "../../lib/utils";
33
import * as sinon from "sinon";
4+
import { expect } from "chai";
5+
6+
const stubInfo = (
7+
redis: Redis,
8+
response: [Error | null, string | undefined]
9+
) => {
10+
sinon.stub(redis, "info").callsFake((section, callback) => {
11+
const cb = typeof section === "function" ? section : callback;
12+
const [error, info] = response;
13+
cb(error, info);
14+
return error ? Promise.reject(error) : Promise.resolve(info);
15+
});
16+
};
417

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

9-
sinon.stub(redis, "info").callsFake((callback) => {
10-
callback(null, "loading:1\r\nloading_eta_seconds:7");
11-
});
22+
stubInfo(redis, [null, "loading:1\r\nloading_eta_seconds:7"]);
23+
1224
// @ts-expect-error
13-
const stub = sinon.stub(global, "setTimeout").callsFake((body, ms) => {
25+
sinon.stub(global, "setTimeout").callsFake((_body, ms) => {
1426
if (ms === 7000) {
15-
redis.info.restore();
16-
stub.restore();
1727
done();
1828
}
1929
});
@@ -29,10 +39,25 @@ describe("ready_check", () => {
2939
},
3040
});
3141

32-
sinon.stub(redis, "info").callsFake((callback) => {
33-
callback(new Error("info error"));
34-
});
42+
stubInfo(redis, [new Error("info error"), undefined]);
3543

3644
redis.connect().catch(noop);
3745
});
46+
47+
it("warns for NOPERM error", async () => {
48+
const redis = new Redis({
49+
lazyConnect: true,
50+
});
51+
52+
const warn = sinon.stub(console, "warn");
53+
stubInfo(redis, [
54+
new Error(
55+
"NOPERM this user has no permissions to run the 'info' command"
56+
),
57+
undefined,
58+
]);
59+
60+
await redis.connect();
61+
expect(warn.calledOnce).to.eql(true);
62+
});
3863
});

0 commit comments

Comments
 (0)
Please sign in to comment.