From 07fec7e18996b6b3b4f30ec9636b88c9b287ece5 Mon Sep 17 00:00:00 2001
From: Nick Gaya <nicholasgaya+github@gmail.com>
Date: Thu, 5 Mar 2020 01:33:55 -0800
Subject: Don't send DISCARD after ExecAbortError in pipeline

The `EXECABORT` error type was added in Redis 2.6.5 and is returned from an
`EXEC` command to indicate that the transaction was aborted due to an invalid
command.  It is not necessary to call `DISCARD` after this error, and doing so
causes a "DISCARD without MULTI" error.
---
 CHANGES                |  2 ++
 redis/client.py        |  2 --
 tests/test_pipeline.py | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/CHANGES b/CHANGES
index 6d3cec6..1eff175 100644
--- a/CHANGES
+++ b/CHANGES
@@ -16,6 +16,8 @@
       during command packing. Thanks @Cody-G. #1265, #1285
     * HSET command now can accept multiple pairs. HMSET has been marked as
       deprecated now. Thanks to @laixintao #1271
+    * Don't manually DISCARD when encountering an ExecAbortError.
+      Thanks @nickgaya, #1300/#1301
 * 3.4.1
     * Move the username argument in the Redis and Connection classes to the
       end of the argument list. This helps those poor souls that specify all
diff --git a/redis/client.py b/redis/client.py
index 10ba979..19707b2 100755
--- a/redis/client.py
+++ b/redis/client.py
@@ -3898,8 +3898,6 @@ class Pipeline(Redis):
         try:
             response = self.parse_response(connection, '_')
         except ExecAbortError:
-            if self.explicit_transaction:
-                self.immediate_execute_command('DISCARD')
             if errors:
                 raise errors[0][1]
             raise sys.exc_info()[1]
diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py
index 828b989..088071b 100644
--- a/tests/test_pipeline.py
+++ b/tests/test_pipeline.py
@@ -172,6 +172,21 @@ class TestPipeline(object):
             assert pipe.set('z', 'zzz').execute() == [True]
             assert r['z'] == b'zzz'
 
+    def test_parse_error_raised_transaction(self, r):
+        with r.pipeline() as pipe:
+            pipe.multi()
+            # the zrem is invalid because we don't pass any keys to it
+            pipe.set('a', 1).zrem('b').set('b', 2)
+            with pytest.raises(redis.ResponseError) as ex:
+                pipe.execute()
+
+            assert unicode(ex.value).startswith('Command # 2 (ZREM b) of '
+                                                'pipeline caused error: ')
+
+            # make sure the pipe was restored to a working state
+            assert pipe.set('z', 'zzz').execute() == [True]
+            assert r['z'] == b'zzz'
+
     def test_watch_succeed(self, r):
         r['a'] = 1
         r['b'] = 2
-- 
cgit v1.2.1