diff --git a/cpp/include/openzl/cpp/Exception.hpp b/cpp/include/openzl/cpp/Exception.hpp index 058260693..06aaeb851 100644 --- a/cpp/include/openzl/cpp/Exception.hpp +++ b/cpp/include/openzl/cpp/Exception.hpp @@ -28,7 +28,7 @@ class Exception : public std::runtime_error { /// @see ExceptionBuilder Exception( poly::string_view msg, - poly::optional code, + poly::optional error, poly::string_view errorContext, poly::source_location location); @@ -37,9 +37,70 @@ class Exception : public std::runtime_error { return msg_; } - const poly::optional& code() const noexcept + poly::optional code() const noexcept { - return code_; + if (error_) { + return ZL_E_code(error_.value()); + } else { + return {}; + } + } + + template + ZL_Error toError( + Ctx* ctx, + poly::source_location this_location = + poly::source_location::current()) const noexcept + { + if (!error_) { + return ZL_E_create( + nullptr, + ZL_GET_DEFAULT_ERROR_CONTEXT(ctx), + location_.file_name(), + location_.function_name(), + location_.line(), + ZL_ErrorCode_GENERIC, + "C++ OpenZL Exception: %s", + what()); + } + auto e = error_.value(); + const auto* opCtx = ZL_GET_OPERATION_CONTEXT(ctx); + if (ZL_OperationContext_ownsError(opCtx, e)) { + e = ZL_E_addFrame( + nullptr, + e, + ZL_EE_EMPTY, + location_.file_name(), + location_.function_name(), + location_.line(), + "Converting to C++ OpenZL Exception: %s", + msg_.c_str()); + e = ZL_E_addFrame( + nullptr, + e, + ZL_EE_EMPTY, + this_location.file_name(), + this_location.function_name(), + this_location.line(), + "Converting back to ZL_Error"); + } + return e; + } + + template + ZL_Error toError( + Ctx& ctx, + poly::source_location this_location = + poly::source_location::current()) const noexcept + { + return toError(ctx.get(), std::move(this_location)); + } + + ZL_Error toError( + poly::source_location this_location = + poly::source_location::current()) const noexcept + { + return toError((ZL_CCtx*)nullptr, std::move(this_location)); } poly::string_view errorContext() const noexcept @@ -54,7 +115,7 @@ class Exception : public std::runtime_error { private: std::string msg_; - poly::optional code_; + poly::optional error_; std::string errorContext_; poly::source_location location_; }; diff --git a/cpp/src/openzl/cpp/Exception.cpp b/cpp/src/openzl/cpp/Exception.cpp index 9e5e48055..bcb9b11a3 100644 --- a/cpp/src/openzl/cpp/Exception.cpp +++ b/cpp/src/openzl/cpp/Exception.cpp @@ -10,7 +10,7 @@ namespace openzl { namespace { std::string formatMsg( std::string_view msg, - const poly::optional& code, + const poly::optional& error, poly::string_view errorContext, const poly::source_location& location) { @@ -21,11 +21,11 @@ std::string formatMsg( out += msg; out += '\n'; } - if (code.has_value()) { + if (error.has_value()) { out += "OpenZL error code: "; - out += std::to_string(code.value()); + out += std::to_string(ZL_E_code(error.value())); out += "\nOpenZL error string: "; - out += ZL_ErrorCode_toString(code.value()); + out += ZL_ErrorCode_toString(ZL_E_code(error.value())); out += "\n"; } if (!errorContext.empty()) { @@ -60,12 +60,12 @@ Exception::Exception(poly::string_view msg, poly::source_location location) Exception::Exception( poly::string_view msg, - poly::optional code, + poly::optional error, poly::string_view errorContext, poly::source_location location) - : std::runtime_error(formatMsg(msg, code, errorContext, location)), + : std::runtime_error(formatMsg(msg, error, errorContext, location)), msg_(msg), - code_(std::move(code)), + error_(std::move(error)), errorContext_(errorContext), location_(std::move(location)) { @@ -148,11 +148,7 @@ ExceptionBuilder&& ExceptionBuilder::addErrorContext( Exception ExceptionBuilder::build() && noexcept { - poly::optional code; - if (error_.has_value()) { - code.emplace(ZL_E_code(error_.value())); - } - return Exception(msg_, code, errorContext_, std::move(location_)); + return Exception(msg_, error_, errorContext_, std::move(location_)); } ZL_Error_Array get_warnings(const ZL_CCtx* const& cctx) diff --git a/cpp/tests/TestException.cpp b/cpp/tests/TestException.cpp index 5637b446f..d6e076885 100644 --- a/cpp/tests/TestException.cpp +++ b/cpp/tests/TestException.cpp @@ -156,4 +156,55 @@ TEST_F(TestException, unwrapWithAllCtxTypes) unwrap(result, "", zl_deserializer); unwrap(result, "", zl_graph); } + +TEST_F(TestException, backToCErrorWithoutContext) +{ + CCtx ctx; + auto result = ZL_CCtx_compress(ctx.get(), NULL, 0, "1234567890", 10); + ASSERT_TRUE(ZL_RES_isError(result)); + const auto orig_code = ZL_E_code(ZL_RES_error(result)); + const std::string orig_str{ ZL_CCtx_getErrorContextString_fromError( + ctx.get(), ZL_RES_error(result)) }; + try { + unwrap(result, "Whoopsie!", ctx.get()); + EXPECT_TRUE(false) << "should be unreachable!"; + } catch (const Exception& ex) { + const auto e = ex.toError(); + const auto code = ZL_E_code(e); + EXPECT_NE(code, ZL_ErrorCode_no_error); + EXPECT_NE(code, ZL_ErrorCode_GENERIC); + EXPECT_EQ(code, orig_code); + const std::string str{ ZL_CCtx_getErrorContextString_fromError( + ctx.get(), e) }; + EXPECT_NE(str.find(orig_str), std::string::npos) << str; + } catch (...) { + EXPECT_TRUE(false) << "shouldn't throw anything else!"; + } +} + +TEST_F(TestException, backToCErrorWithContext) +{ + CCtx ctx; + auto result = ZL_CCtx_compress(ctx.get(), NULL, 0, "1234567890", 10); + ASSERT_TRUE(ZL_RES_isError(result)); + const auto orig_code = ZL_E_code(ZL_RES_error(result)); + const std::string orig_str{ ZL_CCtx_getErrorContextString_fromError( + ctx.get(), ZL_RES_error(result)) }; + try { + unwrap(result, "Whoopsie!", ctx.get()); + EXPECT_TRUE(false) << "should be unreachable!"; + } catch (const Exception& ex) { + const auto e = ex.toError(ctx); + const auto code = ZL_E_code(e); + EXPECT_NE(code, ZL_ErrorCode_no_error); + EXPECT_NE(code, ZL_ErrorCode_GENERIC); + EXPECT_EQ(code, orig_code); + const std::string str{ ZL_CCtx_getErrorContextString_fromError( + ctx.get(), e) }; + EXPECT_NE(str.find("Whoopsie!"), std::string::npos) << str; + EXPECT_NE(str.find(orig_str), std::string::npos) << str; + } catch (...) { + EXPECT_TRUE(false) << "shouldn't throw anything else!"; + } +} } // namespace openzl::tests diff --git a/include/openzl/detail/zl_error_context.h b/include/openzl/detail/zl_error_context.h index 83391e53a..3592ec210 100644 --- a/include/openzl/detail/zl_error_context.h +++ b/include/openzl/detail/zl_error_context.h @@ -168,6 +168,12 @@ ZL_ErrorContext* ZL_OperationContext_getDefaultErrorContext( #define ZL_GET_ERROR_CONTEXT_IMPL(ctx) \ (ZL_OperationContext_getDefaultErrorContext(ZL_GET_OPERATION_CONTEXT(ctx))) +/// @returns true iff the provided rich error info in @p error is owned by this +/// operation context and is still live. +bool ZL_OperationContext_ownsError( + const ZL_OperationContext* opCtx, + ZL_Error error); + #if defined(__cplusplus) } #endif diff --git a/include/openzl/detail/zl_errors_detail.h b/include/openzl/detail/zl_errors_detail.h index 19919dba1..c5b42fbec 100644 --- a/include/openzl/detail/zl_errors_detail.h +++ b/include/openzl/detail/zl_errors_detail.h @@ -11,8 +11,9 @@ #ifndef ZSTRONG_ZS2_ERRORS_DETAIL_H #define ZSTRONG_ZS2_ERRORS_DETAIL_H -#include // size_t -#include // memset +#include // bool +#include // size_t +#include // memset #include "openzl/detail/zl_error_context.h" #include "openzl/zl_errors_types.h" // ZL_ErrorCode diff --git a/src/openzl/common/operation_context.c b/src/openzl/common/operation_context.c index 8f0f35393..6650bcbd8 100644 --- a/src/openzl/common/operation_context.c +++ b/src/openzl/common/operation_context.c @@ -2,6 +2,7 @@ #include "openzl/common/operation_context.h" +#include #include #include @@ -118,14 +119,10 @@ size_t ZL_OC_numWarnings(const ZL_OperationContext* opCtx) return VECTOR_SIZE(opCtx->warnings); } -ZL_DynamicErrorInfo const* ZL_OC_getError( - ZL_OperationContext const* opCtx, - ZL_ErrorCode opCode) +ZL_DynamicErrorInfo const* ZL_OC_getLastError(ZL_OperationContext const* opCtx) { if (opCtx == NULL) return NULL; - if (opCode == ZL_ErrorCode_no_error) - return NULL; if (VECTOR_SIZE(opCtx->errorInfos) == 0) return NULL; ZL_DynamicErrorInfo* dee = @@ -155,7 +152,7 @@ ZL_Error_Array ZL_OC_getWarnings(ZL_OperationContext const* opCtx) .size = size }; } -const char* ZL_OC_getErrorContextString( +ZL_DynamicErrorInfo const* ZL_OC_findError( const ZL_OperationContext* opCtx, ZL_Error error) { @@ -163,24 +160,40 @@ const char* ZL_OC_getErrorContextString( return NULL; } ZL_DynamicErrorInfo* const dy = ZL_E_dy(error); - - // Ensure that the info points into this object or to NULL, we can't trust - // the user. + if (dy == NULL) { + return NULL; + } for (size_t i = 0; i < VECTOR_SIZE(opCtx->errorInfos); i++) { if (dy == VECTOR_AT(opCtx->errorInfos, i)) { - return ZL_E_str(error); + return dy; } } + return NULL; +} + +const char* ZL_OC_getErrorContextString( + const ZL_OperationContext* opCtx, + ZL_Error error) +{ + const ZL_DynamicErrorInfo* const dy = ZL_OC_findError(opCtx, error); + if (dy != NULL) { + return ZL_E_str(error); + } + // TODO: handle static infos? - ZL_LOG(ERROR, - "User passed in a ZL_Report that doesn't belong to this context"); - ZL_E_clearInfo(&error); return "Error does not belong to this context object, you must pass this " "report into the context that created the error (ZL_CCtx for " "compression, ZL_DCtx for decompression, ZL_Compressor for graph " "creation)"; } +bool ZL_OperationContext_ownsError( + const ZL_OperationContext* opCtx, + ZL_Error error) +{ + return ZL_OC_findError(opCtx, error) != NULL; +} + ZL_ErrorContext const* ZL_OC_defaultScopeContext( ZL_OperationContext const* opCtx) { diff --git a/src/openzl/common/operation_context.h b/src/openzl/common/operation_context.h index 2bccd90ef..c9e1fd835 100644 --- a/src/openzl/common/operation_context.h +++ b/src/openzl/common/operation_context.h @@ -90,9 +90,14 @@ size_t ZL_OC_numErrors(const ZL_OperationContext* opCtx); size_t ZL_OC_numWarnings(const ZL_OperationContext* opCtx); /// @returns The current error info or NULL if there is no error. -ZL_DynamicErrorInfo const* ZL_OC_getError( - ZL_OperationContext const* opCtx, - ZL_ErrorCode opCode); +/// Mostly for tests... +ZL_DynamicErrorInfo const* ZL_OC_getLastError(ZL_OperationContext const* opCtx); + +/// @returns a pointer to the error info for the given error if this operation +/// context owns the provided error, NULL otherwise. +ZL_DynamicErrorInfo const* ZL_OC_findError( + const ZL_OperationContext* opCtx, + ZL_Error error); /// @returns The idx'th warning stored in the context. ZL_Error ZL_OC_getWarning(ZL_OperationContext const* opCtx, size_t idx); @@ -102,7 +107,7 @@ ZL_Error_Array ZL_OC_getWarnings(ZL_OperationContext const* opCtx); /// @returns The context string for the provided error, if that error is /// managed by this operation context. Otherwise returns NULL. const char* ZL_OC_getErrorContextString( - const ZL_OperationContext* cctx, + const ZL_OperationContext* opCtx, ZL_Error error); /// @returns The default scope context that points to this operation context. diff --git a/tests/unittest/common/test_errors.cpp b/tests/unittest/common/test_errors.cpp index 77498d310..ef017c94d 100644 --- a/tests/unittest/common/test_errors.cpp +++ b/tests/unittest/common/test_errors.cpp @@ -596,9 +596,7 @@ TEST(ErrorsTest, ErrorInfoWorks) // Check that the fields are set as expected EXPECT_NE(ZL_E_dy(error), nullptr); - EXPECT_EQ( - ZL_E_dy(error), - ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption)); + EXPECT_EQ(ZL_E_dy(error), ZL_OC_getLastError(&opCtx)); EXPECT_EQ(ZL_EE_code(error._info), ZL_ErrorCode_corruption); EXPECT_EQ(ZL_EE_message(error._info), std::string("MyFmtString 350")); EXPECT_EQ(ZL_EE_nbStackFrames(error._info), size_t(1)); @@ -676,9 +674,7 @@ TEST(ErrorsTest, ErrorInfoWorks) 350); EXPECT_NE(ZL_E_dy(error), nullptr); - EXPECT_EQ( - ZL_E_dy(error), - ZL_OC_getError(&opCtx, ZL_ErrorCode_allocation)); + EXPECT_EQ(ZL_E_dy(error), ZL_OC_getLastError(&opCtx)); EXPECT_EQ(ZL_EE_code(error._info), ZL_ErrorCode_allocation); EXPECT_EQ(ZL_EE_message(error._info), std::string("MyFmtString 350")); EXPECT_EQ(ZL_EE_nbStackFrames(error._info), size_t(1)); diff --git a/tests/unittest/common/test_operation_context.cpp b/tests/unittest/common/test_operation_context.cpp index 94899876c..dab45a5ef 100644 --- a/tests/unittest/common/test_operation_context.cpp +++ b/tests/unittest/common/test_operation_context.cpp @@ -70,45 +70,37 @@ TEST(OperationContextTest, BasicUsage) ZL_ErrorContext scopeCtx = { &opCtx }; EXPECT_EQ(ZL_OC_numErrors(&opCtx), 0u); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_no_error), nullptr); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), nullptr); + EXPECT_EQ(ZL_OC_getLastError(&opCtx), nullptr); ZL_OC_startOperation(&opCtx, ZL_Operation_compress); EXPECT_EQ(ZL_OC_numErrors(&opCtx), 0u); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_no_error), nullptr); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), nullptr); + EXPECT_EQ(ZL_OC_getLastError(&opCtx), nullptr); ZL_E_create(nullptr, &scopeCtx, "", "", 0, ZL_ErrorCode_corruption, ""); EXPECT_EQ(ZL_OC_numErrors(&opCtx), 1u); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_no_error), nullptr); - EXPECT_NE(ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), nullptr); - EXPECT_NE(ZL_OC_getError(&opCtx, ZL_ErrorCode_GENERIC), nullptr); - EXPECT_EQ( - ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), - ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption)); + EXPECT_NE(ZL_OC_getLastError(&opCtx), nullptr); ZL_OC_clearErrors(&opCtx); EXPECT_EQ(ZL_OC_numErrors(&opCtx), 0u); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_no_error), nullptr); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), nullptr); + EXPECT_EQ(ZL_OC_getLastError(&opCtx), nullptr); ZL_E_create(nullptr, &scopeCtx, "", "", 0, ZL_ErrorCode_corruption, ""); EXPECT_EQ(ZL_OC_numErrors(&opCtx), 1u); - EXPECT_NE(ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), nullptr); + EXPECT_NE(ZL_OC_getLastError(&opCtx), nullptr); ZL_E_create(nullptr, &scopeCtx, "", "", 0, ZL_ErrorCode_allocation, ""); EXPECT_EQ(ZL_OC_numErrors(&opCtx), 2u); - EXPECT_NE(ZL_OC_getError(&opCtx, ZL_ErrorCode_allocation), nullptr); + EXPECT_NE(ZL_OC_getLastError(&opCtx), nullptr); ZL_OC_startOperation(&opCtx, ZL_Operation_compress); EXPECT_EQ(ZL_OC_numErrors(&opCtx), 0u); - EXPECT_EQ(ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption), nullptr); + EXPECT_EQ(ZL_OC_getLastError(&opCtx), nullptr); ZL_OC_destroy(&opCtx); } @@ -141,7 +133,7 @@ TEST(OperationContextTest, Warnings) auto dy1 = ZL_E_dy(e1); EXPECT_NE(dy1, nullptr); - EXPECT_EQ(dy1, ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption)); + EXPECT_EQ(dy1, ZL_OC_getLastError(&opCtx)); ZL_OC_markAsWarning(&opCtx, e1); @@ -167,7 +159,7 @@ TEST(OperationContextTest, Warnings) auto dy2 = ZL_E_dy(e2); EXPECT_NE(dy2, nullptr); - EXPECT_EQ(dy2, ZL_OC_getError(&opCtx, ZL_ErrorCode_corruption)); + EXPECT_EQ(dy2, ZL_OC_getLastError(&opCtx)); ZL_OC_markAsWarning(&opCtx, e2);