From b0c36084cfd2d0d2832b68c861d0db5c679e4d2e Mon Sep 17 00:00:00 2001 From: moticless Date: Sat, 16 Mar 2024 20:45:22 +0200 Subject: [PATCH] Aggregate reported-errors up to 2kb --- src/lib/parser.c | 24 +++++++++++++++++++----- src/lib/parser.h | 9 ++++++++- test/test_common.c | 5 ++++- test/test_common.h | 2 ++ test/test_main.c | 45 +++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/lib/parser.c b/src/lib/parser.c index 76d6210..9e8e6bc 100644 --- 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 index a2a2bbf..a725f7c 100644 --- 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 index 93c45e1..1fcecf3 100644 --- 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 index 50b13c4..a5d486a 100644 --- 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 index ffbdf56..982f829 100644 --- 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); }