Skip to content

Commit

Permalink
Include all runtime errors in transaction result
Browse files Browse the repository at this point in the history
Some transaction run time errors like WRONGTYPE will get QUEUED, and
therefore the error is included in the final response array. Other
errors however like READONLY dont get queued but are rejected straight
away, and Redis will therefore exclude them from the exec result. This
change reinserts such errors into the right position in the exec()
result.
  • Loading branch information
alavers committed Nov 25, 2019
1 parent 9bda593 commit 8e84557
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 0 deletions.
16 changes: 16 additions & 0 deletions lib/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,22 @@ export function addTransactionSupport(redis) {
}
throw execResult[0];
}
// Minus two to exclude the multi and exec responses
const commandCount = result.length - 2;
if (execResult[1] !== null && execResult[1].length !== commandCount) {
// Some commands failed and the errors are not included in execResult[1]
// Merge in those failures in the correct order
const mergedResult = [];
let execResultIndex = 0;
for (let i = 1; i < result.length - 1; ++i) {
if (result[i][0]) {
mergedResult.push(result[i][0]);
} else {
mergedResult.push(execResult[1][execResultIndex++]);
}
}
return wrapMultiResult(mergedResult);
}
return wrapMultiResult(execResult[1]);
}),
callback
Expand Down
57 changes: 57 additions & 0 deletions test/functional/transaction.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
import Redis from "../../lib/redis";
import { ReplyError } from "../../lib";
import { expect } from "chai";
import Command from "../../lib/command";
import MockServer from "../helpers/mock_server";

const READONLY_ERROR = "READONLY You can't write against a read only replica.";

function simulateReadOnlyReplica() {
let pendingResults = [];
let execResults = [];
new MockServer(30000, argv => {
switch (argv[0]) {
case "get":
pendingResults.push("bar");
return MockServer.raw("+QUEUED\r\n");
case "set":
return new Error(READONLY_ERROR);
case "exec":
execResults = pendingResults;
pendingResults = [];
return execResults;
}
});

return new Redis({
port: 30000
});
}

describe("transaction", function() {
it("should works like pipeline by default", function(done) {
Expand Down Expand Up @@ -47,6 +73,37 @@ describe("transaction", function() {
});
});

it("should handle readonly errors correctly", async function() {
const redis = simulateReadOnlyReplica();

const result = await redis
.multi()
.set("foo", "bar")
.exec();
expect(result.length).to.eql(1);
expect(result[0][0]).to.be.instanceof(ReplyError);
expect(result[0][0].toString()).to.eql(`ReplyError: ${READONLY_ERROR}`);
});

it("should reassemble partially failed results in the correct order", async function() {
const redis = simulateReadOnlyReplica();

const result = await redis
.multi()
.get("foo")
.set("foo", "bar")
.get("foo")
.set("foo", "bar")
.exec();
expect(result.length).to.eql(4);
expect(result[0]).to.eql([null, "bar"]);
expect(result[1][0]).to.be.instanceof(ReplyError);
expect(result[1][0].toString()).to.eql(`ReplyError: ${READONLY_ERROR}`);
expect(result[2]).to.eql([null, "bar"]);
expect(result[3][0]).to.be.instanceof(ReplyError);
expect(result[3][0].toString()).to.eql(`ReplyError: ${READONLY_ERROR}`);
});

it("should also support command callbacks", function(done) {
var redis = new Redis();
var pending = 1;
Expand Down

0 comments on commit 8e84557

Please sign in to comment.