From 20fa156067203b93b7050cc444430d31a6cb6e1b Mon Sep 17 00:00:00 2001
From: guybe7 <guy.benoish@redislabs.com>
Date: Tue, 20 Jun 2023 19:44:43 +0200
Subject: [PATCH] Align RM_ReplyWithErrorFormat with RM_ReplyWithError (#12321)

Introduced by https://github.com/redis/redis/pull/11923 (Redis 7.2 RC2)

It's very weird and counterintuitive that `RM_ReplyWithError` requires the error-code
**without** a hyphen while `RM_ReplyWithErrorFormat` requires either the error-code
**with** a hyphen or no error-code at all
```
RedisModule_ReplyWithError(ctx, "BLA bla bla");
```
vs.
```
RedisModule_ReplyWithErrorFormat(ctx, "-BLA %s", "bla bla");
```

This commit aligns RM_ReplyWithErrorFormat to behvae like RM_ReplyWithError.
it's a breaking changes but it's done before 7.2 goes GA.
---
 src/module.c                   | 20 +++++++++++++-------
 tests/unit/moduleapi/reply.tcl | 12 +++++++++---
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/module.c b/src/module.c
index 57e84e712..e41b275c2 100644
--- a/src/module.c
+++ b/src/module.c
@@ -2998,15 +2998,15 @@ int RM_ReplyWithError(RedisModuleCtx *ctx, const char *err) {
 
 /* Reply with the error create from a printf format and arguments.
  *
- * If the error code is already passed in the string 'fmt', the error
- * code provided is used, otherwise the string "-ERR " for the generic
- * error code is automatically added.
+ * Note that 'fmt' must contain all the error, including
+ * the initial error code. The function only provides the initial "-", so
+ * the usage is, for example:
  *
- * The usage is, for example:
+ *     RedisModule_ReplyWithErrorFormat(ctx,"ERR Wrong Type: %s",type);
  *
- *     RedisModule_ReplyWithErrorFormat(ctx, "An error: %s", "foo");
+ * and not just:
  *
- *     RedisModule_ReplyWithErrorFormat(ctx, "-WRONGTYPE Wrong Type: %s", "foo");
+ *     RedisModule_ReplyWithErrorFormat(ctx,"Wrong Type: %s",type);
  *
  * The function always returns REDISMODULE_OK.
  */
@@ -3014,11 +3014,17 @@ int RM_ReplyWithErrorFormat(RedisModuleCtx *ctx, const char *fmt, ...) {
     client *c = moduleGetReplyClient(ctx);
     if (c == NULL) return REDISMODULE_OK;
 
+    int len = strlen(fmt) + 2; /* 1 for the \0 and 1 for the hyphen */
+    char *hyphenfmt = zmalloc(len);
+    snprintf(hyphenfmt, len, "-%s", fmt);
+
     va_list ap;
     va_start(ap, fmt);
-    addReplyErrorFormatInternal(c, 0, fmt, ap);
+    addReplyErrorFormatInternal(c, 0, hyphenfmt, ap);
     va_end(ap);
 
+    zfree(hyphenfmt);
+
     return REDISMODULE_OK;
 }
 
diff --git a/tests/unit/moduleapi/reply.tcl b/tests/unit/moduleapi/reply.tcl
index 547be21c0..3cf284de3 100644
--- a/tests/unit/moduleapi/reply.tcl
+++ b/tests/unit/moduleapi/reply.tcl
@@ -128,13 +128,19 @@ start_server {tags {"modules"}} {
 
         test "RESP$proto: RM_ReplyWithErrorFormat: error format reply" {
             catch {r rw.error_format "An error: %s" foo} e
-            assert_match "ERR An error: foo" $e
+            assert_match "An error: foo" $e  ;# Should not be used by a user, but compatible with RM_ReplyError
 
             catch {r rw.error_format "-ERR An error: %s" foo2} e
-            assert_match "ERR An error: foo2" $e
+            assert_match "-ERR An error: foo2" $e  ;# Should not be used by a user, but compatible with RM_ReplyError (There are two hyphens, TCL removes the first one)
 
             catch {r rw.error_format "-WRONGTYPE A type error: %s" foo3} e
-            assert_match "WRONGTYPE A type error: foo3" $e
+            assert_match "-WRONGTYPE A type error: foo3" $e  ;# Should not be used by a user, but compatible with RM_ReplyError (There are two hyphens, TCL removes the first one)
+
+            catch {r rw.error_format "ERR An error: %s" foo4} e
+            assert_match "ERR An error: foo4" $e
+
+            catch {r rw.error_format "WRONGTYPE A type error: %s" foo5} e
+            assert_match "WRONGTYPE A type error: foo5" $e
         }
 
         r hello 2