Aggregate reported-errors up to 2kb

moticless · Mar 17, 2024 · b0c3608 · b0c3608
1 parent df07dbc
commit b0c3608
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 9 deletions.
diff --git a/src/lib/parser.c b/src/lib/parser.c
@@ -207,6 +207,7 @@ _LIBRDB_API RdbParser *RDB_createParserRdb(RdbMemAlloc *memAlloc) {
     p->reader = NULL;
     p->cache = NULL;
     p->errorMsg[0] = '\0';
+    p->errorMsgAt = 0;
     p->appCbCtx.numBulks = 0;
     p->loggerCb = loggerCbDefault;
     p->logLevel = RDB_LOG_DBG;
@@ -447,28 +448,41 @@ _LIBRDB_API RdbRes RDB_getErrorCode(RdbParser *p) {
 }
 
 _LIBRDB_API void RDB_reportError(RdbParser *p, RdbRes e, const char *msg, ...) {
-    int nchars = 0;
-    p->errorCode = e;
+    /* Record errorCode only of the first error */
+    if (p->errorCode == RDB_OK)
+        p->errorCode = e;
 
     if (msg == NULL) {
-        p->errorMsg[0] = '\0';
         return;
     }
 
     /* RDB_OK & RDB_OK_DONT_PROPAGATE - not a real errors to report */
     assert (e != RDB_OK && e != RDB_OK_DONT_PROPAGATE);
 
+    /* If error message is too long, then trim it in order to record this last message */
+    if (p->errorMsgAt > LAST_ERR_MSG_OFFSET) {
+        p->errorMsgAt = LAST_ERR_MSG_OFFSET;
+        p->errorMsgAt += snprintf(p->errorMsg + p->errorMsgAt,
+                                  MAX_ERROR_MSG - p->errorMsgAt,
+                                  "\n... last recorded error message: ...\n");
+    }
+    p->errorMsgAt += snprintf(p->errorMsg + p->errorMsgAt, MAX_ERROR_MSG - p->errorMsgAt, "[errcode=%d] ", e);
+
     if (p->state == RDB_STATE_RUNNING) {
-        nchars = snprintf(p->errorMsg, MAX_ERROR_MSG, "[%s::State=%d] ",
+        p->errorMsgAt += snprintf(p->errorMsg + p->errorMsgAt,
+                          MAX_ERROR_MSG - p->errorMsgAt, "[%s::State=%d] ",
                           peInfo[p->parsingElement].funcname,
                           p->elmCtx.state);
     }
 
     va_list args;
     va_start(args, msg);
-    vsnprintf(p->errorMsg + nchars, MAX_ERROR_MSG - nchars, msg, args);
+    p->errorMsgAt += vsnprintf(p->errorMsg + p->errorMsgAt, MAX_ERROR_MSG - p->errorMsgAt, msg, args);
     va_end(args);
 
+    if (p->errorMsgAt >= MAX_ERROR_MSG) return;
+    p->errorMsgAt += snprintf(p->errorMsg + p->errorMsgAt, MAX_ERROR_MSG - p->errorMsgAt, "\n");
+
     RDB_log(p, RDB_LOG_ERR, "%s", p->errorMsg);
 }
 

diff --git a/src/lib/parser.h b/src/lib/parser.h
@@ -6,7 +6,12 @@
 #include "defines.h"
 #include "../../api/librdb-api.h"
 
-#define MAX_ERROR_MSG 1024
+
+/* Max error message length. Chain one or more recorded error messages. */
+#define MAX_ERROR_MSG 2048
+/* When reaching configured offset, keep overwrite the last error message */
+#define LAST_ERR_MSG_OFFSET 1600
+
 #define MAX_APP_BULKS 2
 #define NOP /*no-op*/
 #define IF_NOT_OK_RETURN(cmd) do {RdbStatus s; s = cmd; if (unlikely(s!=RDB_STATUS_OK)) return s;} while (0)
@@ -373,7 +378,9 @@ struct RdbParser {
 
     /*** error reporting ***/
     RdbRes errorCode;
+
     char errorMsg[MAX_ERROR_MSG];
+    int errorMsgAt;
 
     /*** read RDB from reader VS read RDB from buffer ***/
 

diff --git a/test/test_common.c b/test/test_common.c
@@ -105,7 +105,10 @@ void cleanTmpFolder(void) {
     const char *folder_path = "./test/tmp";
 
     DIR *dir = opendir(folder_path);
-    assert_true(dir != NULL);
+    if (dir == NULL) {
+        printf("Failed to open directory: %s\n", folder_path);
+        assert_true(0);
+    }
 
     struct dirent *entry;
     while ((entry = readdir(dir)) != NULL) {

diff --git a/test/test_common.h b/test/test_common.h
@@ -76,4 +76,6 @@ void setEnvVar (const char *name, const char *val);
 char *substring(char *str, size_t len, char *substr);
 void assert_file_payload(const char *filename, char *expData, int expLen, MatchType matchType, int expMatch);
 
+void dummyLogger(RdbLogLevel l, const char *msg);
+
 int printHexDump(const char *addr, size_t len, char *obuf, int obuflen);
diff --git a/test/test_main.c b/test/test_main.c
@@ -18,8 +18,7 @@ static void test_createReader_missingFile(void **state) {
     assert_int_equal(err, RDB_ERR_FAILED_OPEN_RDB_FILE);
 
     /* verify returned error string */
-    assert_string_equal(RDB_getErrorMessage(parser),
-                        "Failed to open RDB file `./test/dumps/non_exist_file.rdb`: No such file or directory\n");
+    assert_true(strstr(RDB_getErrorMessage(parser), "Failed to open RDB file"));
     RDB_deleteParser(parser);
 }
 
@@ -110,6 +109,47 @@ static void test_examples(void **state) {
     runSystemCmd("make example > /dev/null ");
 }
 
+RdbRes handle_start_rdb_report_long_errors(RdbParser *p, void *userData, int rdbVersion) {
+    UNUSED(userData, rdbVersion);
+    for (int i = 1 ; i < 1000; i++)
+        RDB_reportError(p, (RdbRes) i, "Error Report number:%d", i);
+    return 1001; /* This value will be eventually returned as the error code */
+}
+static void test_report_long_error(void **state) {
+    RdbStatus  status;
+    UNUSED(state);
+    void *user_data = NULL;
+
+    RdbHandlersRawCallbacks cb = { .handleStartRdb = handle_start_rdb_report_long_errors };
+    RdbParser *parser = RDB_createParserRdb(NULL);
+    RDB_setLogger(parser, dummyLogger);
+    assert_non_null(RDBX_createReaderFile(parser, "./test/dumps/quicklist2_v11.rdb"));
+    assert_non_null(RDB_createHandlersRaw(parser, &cb, user_data, NULL));
+
+
+    while ((status = RDB_parse(parser)) == RDB_STATUS_WAIT_MORE_DATA);
+    assert_int_equal(status, RDB_STATUS_ERROR);
+    const char *returned = RDB_getErrorMessage(parser);
+    const char *expected =
+        "[errcode=1] [elementRdbHeader::State=0] Error Report number:1\n[errcode=2] [elementRdbHeader::State=0] Error Report number:2\n"
+        "[errcode=3] [elementRdbHeader::State=0] Error Report number:3\n[errcode=4] [elementRdbHeader::State=0] Error Report number:4\n"
+        "[errcode=5] [elementRdbHeader::State=0] Error Report number:5\n[errcode=6] [elementRdbHeader::State=0] Error Report number:6\n"
+        "[errcode=7] [elementRdbHeader::State=0] Error Report number:7\n[errcode=8] [elementRdbHeader::State=0] Error Report number:8\n"
+        "[errcode=9] [elementRdbHeader::State=0] Error Report number:9\n[errcode=10] [elementRdbHeader::State=0] Error Report number:10\n"
+        "[errcode=11] [elementRdbHeader::State=0] Error Report number:11\n[errcode=12] [elementRdbHeader::State=0] Error Report number:12\n"
+        "[errcode=13] [elementRdbHeader::State=0] Error Report number:13\n[errcode=14] [elementRdbHeader::State=0] Error Report number:14\n"
+        "[errcode=15] [elementRdbHeader::State=0] Error Report number:15\n[errcode=16] [elementRdbHeader::State=0] Error Report number:16\n"
+        "[errcode=17] [elementRdbHeader::State=0] Error Report number:17\n[errcode=18] [elementRdbHeader::State=0] Error Report number:18\n"
+        "[errcode=19] [elementRdbHeader::State=0] Error Report number:19\n[errcode=20] [elementRdbHeader::State=0] Error Report number:20\n"
+        "[errcode=21] [elementRdbHeader::State=0] Error Report number:21\n[errcode=22] [elementRdbHeader::State=0] Error Report number:22\n"
+        "[errcode=23] [elementRdbHeader::State=0] Error Report number:23\n[errcode=24] [elementRdbHeader::State=0] Error Report number:24\n"
+        "[errcode=25] [elementRdbHeader::State=0] Error Report number:25\n[errcode=26] [elem\n... last recorded error message: ...\n"
+        "[errcode=999] [elementRdbHeader::State=0] Error Report number:999\n";
+    assert_string_equal(returned, expected);
+    assert_int_equal(RDB_getErrorCode(parser), 1001);
+    RDB_deleteParser(parser);
+}
+
 static void printResPicture(int result) {
     if (result)
         printf("    x_x\n"
@@ -153,6 +193,7 @@ int group_misc(void) {
         cmocka_unit_test(test_empty_rdb),
         cmocka_unit_test(test_mixed_levels_registration),
         cmocka_unit_test(test_checksum),
+        cmocka_unit_test(test_report_long_error),
     };
     return cmocka_run_group_tests(tests, NULL, NULL);
 }