AUTH Failure returns instead of using callback

[cressie176]

I'd like to use the AUTH callback to be notified of invalid passwords, however instead of passing the callback and error.

            if (err.toString().match("LOADING")) {
                // if redis is still loading the db, it will not authenticate and everything else will fail
                console.log("Redis still loading, trying to authenticate later");
                setTimeout(function () {
                    self.do_auth();
                }, 2000); // TODO - magic number alert
                return;
            } else {
                // INVALID PASSWORD PATH ENTERS HERE AND RETURNS
                return self.emit("error", new Error("Auth error: " + err.message));
            }
// THE FOLLOWING CODE IS NOT EXECUTED
        if (res.toString() !== "OK") {
            return self.emit("error", new Error("Auth failed: " + res.toString()));
        }
        if (exports.debug_mode) {
            console.log("Auth succeeded " + self.host + ":" + self.port + " id " + self.connection_id);
        }
        if (self.auth_callback) {
            self.auth_callback(err, res);
            self.auth_callback = null;
        }

Any chance of changing the above to so it calls self.auth_callback if present on any error? Happy to fork and send a pull request.

Steve

[brycebaril]

Good catch -- this definitely looks like a bug. It would be awesome if you sent a pull request!

[cressie176]

Looks like the change is more complicated than I first appreciated. It's easy enough to restructure the code so auth_callback is still called on error, but if the auth_callback runs before the onReady check any redis commands sent within callback fail (unless the callback set the "secret" send_anyway command).

Moving the auth_callback after the onReady check presumably breaks the original intent, and might lead to undesired side-effects since the "connect" event would be emitted before the auth had been performed.

There also seems to be some instances of synchronous code running after asynchronous code, that might result in indeterministic behaviour. e.g.

        // Contents of the callback is potentially async
        if (self.auth_callback) {
            self.auth_callback(err, res);
            self.auth_callback = null;
        }

        // If the contents of the callback is async might the following code run before the callback completes?
        self.emit("connect");
        if (self.options.no_ready_check) {
            self.on_ready();
        } else {
            self.ready_check();
        }

Any thoughts?

[narcisoguillen]

How about doing

return self.auth_callback(new Error("Auth error: " + err.message));

instead of

return self.emit("error", new Error("Auth error: " + err.message));

I think is a simple way of executing the auth callback

[BridgeAR]

Reopened as this is not fixed yet and seems to be a bug.