WATCH during MULTI shouldn't fail transaction

[jabolina]

Bug Report

Redis returns an error if a WATCH command is submitted inside a MULTI. However, this command is silently discarded. The WATCH command does not count in the final result list during the EXEC phase. For example:

127.0.0.1:6379> multi
OK
127.0.0.1:6379(TX)> set foo fooer
QUEUED
127.0.0.1:6379(TX)> watch some-key
(error) ERR WATCH inside MULTI is not allowed
127.0.0.1:6379(TX)> set bar baer
QUEUED
127.0.0.1:6379(TX)> exec
1) OK
2) OK

Observe that the WATCH command is not included in the result list.

Current Behavior

Lettuce includes the failed WATCH command in the output list. Additionally, since the command is dropped, the response list has fewer responses than the command list in MultiOutput and some commands might never be completed.

Input Code

For example, adding the following test to the TransactionCommandIntegrationTests class:

Input Code

    @Test
    void watchWithinMulti() {
        redis.multi();
        redis.set("one-a", "a");
        redis.set("one-b", "b");
        redis.watch("something");
        redis.set("one-c", "c");

        TransactionResult result = redis.exec();
        for (Object o : result) {
            System.out.println(o);
        }

        assertThat(result)
                .hasSize(3)
                .allMatch("OK"::equals);
    }

Produces:

OK
OK
io.lettuce.core.RedisCommandExecutionException: ERR WATCH inside MULTI is not allowed

java.lang.AssertionError: 
Expecting all elements of:
  DefaultTransactionResult [wasRolledBack=false, responses=3]
to match given predicate but this element did not:
  io.lettuce.core.RedisCommandExecutionException: ERR WATCH inside MULTI is not allowed
	at io.lettuce.core.internal.ExceptionFactory.createExecutionException(ExceptionFactory.java:152)
	at io.lettuce.core.internal.ExceptionFactory.createExecutionException(ExceptionFactory.java:120)
	at io.lettuce.core.output.MultiOutput.complete(MultiOutput.java:154)
	...(25 remaining lines not displayed - this can be changed with Assertions.setMaxStackTraceElementsDisplayed)

	at io.lettuce.core.commands.TransactionCommandIntegrationTests.watchWithinMulti(TransactionCommandIntegrationTests.java:135)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Expected behavior/code

The test should complete successfully with all 3 set operations returning an OK response.

Environment

• Lettuce version(s): main branch

Additional context

I think WATCH is the only command that the MULTI will drop. The solution will require something specific to WATCH.

[tishun]

I think WATCH is the only command that the MULTI will drop. The solution will require something specific to WATCH.

Not only, unfortunately. MULTI inside MULTI is not allowed too (and does not fail the transaction) :

127.0.0.1:6484> MULTI
OK
127.0.0.1:6484(TX)> SET a b
QUEUED
127.0.0.1:6484(TX)> MULTI
(error) ERR MULTI calls can not be nested
127.0.0.1:6484(TX)> SET b b
QUEUED
127.0.0.1:6484(TX)> EXEC
1) OK
2) OK

[tishun]

The test should complete successfully with all 3 set operations returning an OK response.

I agree that the last result should not be omitted from the TransactionResult, but I disagree that the error should be silently ignored. After all the transaction was supposed to contain 4 commands and, respectively, I would assume it would have 4 results (for each of the commands). Otherwise it might not be immediately visible to the user where the result from the WATCH command is, or why it failed for that matter.