From 136d309dd4bc1ada9f3d775f036b62292fda390b Mon Sep 17 00:00:00 2001 From: David Steele Date: Fri, 1 Oct 2021 15:29:31 -0400 Subject: [PATCH] Allow stack trace to be specified for errorInternalThrow(). This allows the stack trace to be set when an error is received by the protocol, rather than appending it to the message. Now these errors will look no different than any other error and the stack trace will be reported in the same way. One immediate benefit is that test.pl --vm-out --log-level-test=debug will work for tests that check expect log results. Previously, the test would error at the first check because the stack trace included in the message would not match the expected log output. --- src/common/error.c | 23 +++++++++++----- src/common/error.h | 10 +++---- src/protocol/client.c | 17 +++--------- test/define.yaml | 2 +- test/src/module/common/errorTest.c | 16 +++++++++++ test/src/module/protocol/protocolTest.c | 36 +++++++++++-------------- 6 files changed, 57 insertions(+), 47 deletions(-) diff --git a/src/common/error.c b/src/common/error.c index fccb3acda..2cded79a3 100644 --- a/src/common/error.c +++ b/src/common/error.c @@ -379,7 +379,9 @@ errorInternalProcess(bool catch) /**********************************************************************************************************************************/ void -errorInternalThrow(const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) +errorInternalThrow( + const ErrorType *const errorType, const char *const fileName, const char *const functionName, const int fileLine, + const char *const message, const char *const stackTrace) { // Setup error data errorContext.error.errorType = errorType; @@ -393,8 +395,15 @@ errorInternalThrow(const ErrorType *errorType, const char *fileName, const char errorContext.error.message = (const char *)messageBuffer; - // Generate the stack trace for the error - if (stackTraceToZ( + // If a stack trace was provided + if (stackTrace != NULL) + { + strncpy(stackTraceBuffer, stackTrace, sizeof(stackTraceBuffer) - 1); + messageBuffer[sizeof(stackTraceBuffer) - 1] = '\0'; + } + // Else generate the stack trace for the error + else if ( + stackTraceToZ( stackTraceBuffer, sizeof(stackTraceBuffer), fileName, functionName, (unsigned int)fileLine) >= sizeof(stackTraceBuffer)) { // Indicate that the stack trace was truncated @@ -416,7 +425,7 @@ errorInternalThrowFmt( vsnprintf(messageBufferTemp, ERROR_MESSAGE_BUFFER_SIZE - 1, format, argument); va_end(argument); - errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp); + errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp, NULL); } /**********************************************************************************************************************************/ @@ -433,7 +442,7 @@ errorInternalThrowSys( else snprintf(messageBufferTemp, ERROR_MESSAGE_BUFFER_SIZE - 1, "%s: [%d] %s", message, errNo, strerror(errNo)); - errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp); + errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp, NULL); } #ifdef DEBUG_COVERAGE @@ -461,7 +470,7 @@ errorInternalThrowSysFmt( if (errNo != 0) snprintf(messageBufferTemp + messageSize, ERROR_MESSAGE_BUFFER_SIZE - 1 - messageSize, ": [%d] %s", errNo, strerror(errNo)); - errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp); + errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp, NULL); } #ifdef DEBUG_COVERAGE @@ -485,7 +494,7 @@ errorInternalThrowOnSysFmt( messageBufferTemp + messageSize, ERROR_MESSAGE_BUFFER_SIZE - 1 - messageSize, ": [%d] %s", errNo, strerror(errNo)); } - errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp); + errorInternalThrow(errorType, fileName, functionName, fileLine, messageBufferTemp, NULL); } } #endif diff --git a/src/common/error.h b/src/common/error.h index 40225e3bb..237fe9cdc 100644 --- a/src/common/error.h +++ b/src/common/error.h @@ -167,16 +167,16 @@ error information to stderr. The seldom used "THROWP" variants allow an error to be thrown with a pointer to the error type. ***********************************************************************************************************************************/ #define THROW(errorType, message) \ - errorInternalThrow(&errorType, __FILE__, __func__, __LINE__, message) + errorInternalThrow(&errorType, __FILE__, __func__, __LINE__, message, NULL) #define THROW_FMT(errorType, ...) \ errorInternalThrowFmt(&errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) #define THROWP(errorType, message) \ - errorInternalThrow(errorType, __FILE__, __func__, __LINE__, message) + errorInternalThrow(errorType, __FILE__, __func__, __LINE__, message, NULL) #define THROWP_FMT(errorType, ...) \ errorInternalThrowFmt(errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) #define THROW_CODE(errorCode, message) \ - errorInternalThrow(errorTypeFromCode(errorCode), __FILE__, __func__, __LINE__, message) + errorInternalThrow(errorTypeFromCode(errorCode), __FILE__, __func__, __LINE__, message, NULL) #define THROW_CODE_FMT(errorCode, ...) \ errorInternalThrowFmt(errorTypeFromCode(errorCode), __FILE__, __func__, __LINE__, __VA_ARGS__) @@ -301,8 +301,8 @@ void errorInternalPropagate(void) __attribute__((__noreturn__)); // Throw an error void errorInternalThrow( - const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) - __attribute__((__noreturn__)); + const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message, + const char *stackTrace) __attribute__((__noreturn__)); void errorInternalThrowFmt( const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) __attribute__((format(printf, 5, 6))) __attribute__((__noreturn__)); diff --git a/src/protocol/client.c b/src/protocol/client.c index cadb7c5f2..3bccd6901 100644 --- a/src/protocol/client.c +++ b/src/protocol/client.c @@ -181,23 +181,14 @@ protocolClientError(ProtocolClient *const this, const ProtocolMessageType type, { if (type == protocolMessageTypeError) { - const ErrorType *type = errorTypeFromCode(pckReadI32P(error)); - String *const message = strNewFmt("%s: %s", strZ(this->errorPrefix), strZ(pckReadStrP(error))); + const ErrorType *const type = errorTypeFromCode(pckReadI32P(error)); + const String *const message = strNewFmt("%s: %s", strZ(this->errorPrefix), strZ(pckReadStrP(error))); const String *const stack = pckReadStrP(error); pckReadEndP(error); - CHECK(message != NULL); + CHECK(message != NULL && stack != NULL); - // Add stack trace if the error is an assertion or debug-level logging is enabled - if (type == &AssertError || logAny(logLevelDebug)) - { - CHECK(stack != NULL); - - strCat(message, LF_STR); - strCat(message, stack); - } - - THROWP(type, strZ(message)); + errorInternalThrow(type, __FILE__, __func__, __LINE__, strZ(message), strZ(stack)); } } MEM_CONTEXT_TEMP_END(); diff --git a/test/define.yaml b/test/define.yaml index 2f1998363..0a551ee36 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -46,7 +46,7 @@ unit: test: # ---------------------------------------------------------------------------------------------------------------------------- - name: error - total: 8 + total: 9 feature: error harness: name: error diff --git a/test/src/module/common/errorTest.c b/test/src/module/common/errorTest.c index ab9445bff..ec41a2237 100644 --- a/test/src/module/common/errorTest.c +++ b/test/src/module/common/errorTest.c @@ -107,6 +107,22 @@ testRun(void) assert(errorContext.tryTotal == 0); } + // ***************************************************************************************************************************** + if (testBegin("errorInternalThrow()")) + { + TEST_TITLE("specify all parameters"); + + TRY_BEGIN() + { + errorInternalThrow(&FormatError, "file77", "function88", 99, "message100", "stacktrace200"); + } + CATCH_ANY() + { + assert(errorType() == &FormatError); + } + TRY_END(); + } + // ***************************************************************************************************************************** if (testBegin("TRY with multiple catches")) { diff --git a/test/src/module/protocol/protocolTest.c b/test/src/module/protocol/protocolTest.c index 71b19a808..c28bbc39a 100644 --- a/test/src/module/protocol/protocolTest.c +++ b/test/src/module/protocol/protocolTest.c @@ -595,27 +595,21 @@ testRun(void) // ----------------------------------------------------------------------------------------------------------------- TEST_TITLE("command throws assert"); - TEST_ERROR( - protocolClientExecute(client, protocolCommandNew(TEST_PROTOCOL_COMMAND_ASSERT), false), AssertError, - "raised from test client: ERR_MESSAGE\nERR_STACK_TRACE"); - - // ----------------------------------------------------------------------------------------------------------------- - TEST_TITLE("command throws error"); - - TEST_ERROR( - protocolClientExecute(client, protocolCommandNew(TEST_PROTOCOL_COMMAND_ERROR), false), FormatError, - "raised from test client: ERR_MESSAGE"); - - // ----------------------------------------------------------------------------------------------------------------- - TEST_TITLE("command throws error in debug log level"); - - harnessLogLevelSet(logLevelDebug); - - TEST_ERROR( - protocolClientExecute(client, protocolCommandNew(TEST_PROTOCOL_COMMAND_ERROR), false), FormatError, - "raised from test client: ERR_MESSAGE\nERR_STACK_TRACE"); - - harnessLogLevelReset(); + TRY_BEGIN() + { + protocolClientExecute(client, protocolCommandNew(TEST_PROTOCOL_COMMAND_ASSERT), false); + THROW(TestError, "error was expected"); + } + CATCH_ANY() + { + TEST_RESULT_PTR(errorType(), &AssertError, "check type"); + TEST_RESULT_Z(errorFileName(), TEST_PGB_PATH "/src/protocol/client.c", "check file"); + TEST_RESULT_Z(errorFunctionName(), "protocolClientError", "check function"); + TEST_RESULT_BOOL(errorFileLine() > 0, true, "check file line > 0"); + TEST_RESULT_Z(errorMessage(), "raised from test client: ERR_MESSAGE", "check message"); + TEST_RESULT_Z(errorStackTrace(), "ERR_STACK_TRACE", "check stack trace"); + } + TRY_END(); // ----------------------------------------------------------------------------------------------------------------- TEST_TITLE("noop command");