From 8466d7ea3b91f610d22634a02bd44d26d8345c3d Mon Sep 17 00:00:00 2001 From: Felix Handte Date: Thu, 30 Apr 2026 10:15:39 -0700 Subject: [PATCH 1/4] C++: Easy: Store Whole ZL_Error in Exception Summary: As title. Differential Revision: D101645553 --- cpp/include/openzl/cpp/Exception.hpp | 12 ++++++++---- cpp/src/openzl/cpp/Exception.cpp | 20 ++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cpp/include/openzl/cpp/Exception.hpp b/cpp/include/openzl/cpp/Exception.hpp index 058260693..837567835 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,13 @@ 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 {}; + } } poly::string_view errorContext() const noexcept @@ -54,7 +58,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) From b064bf337c6dc230683184db5f05cd5470f26cf1 Mon Sep 17 00:00:00 2001 From: Felix Handte Date: Thu, 30 Apr 2026 10:15:39 -0700 Subject: [PATCH 2/4] Simplify `ZL_OC_getLastError()` Summary: This function maybe shouldn't exist. But we do use it in tests. At least simplify it by getting rid of this weird arg. Reviewed By: terrelln Differential Revision: D101645608 --- src/openzl/common/operation_context.c | 6 +---- src/openzl/common/operation_context.h | 5 ++-- tests/unittest/common/test_errors.cpp | 8 ++---- .../common/test_operation_context.cpp | 26 +++++++------------ 4 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/openzl/common/operation_context.c b/src/openzl/common/operation_context.c index 8f0f35393..2ea2b4a1d 100644 --- a/src/openzl/common/operation_context.c +++ b/src/openzl/common/operation_context.c @@ -118,14 +118,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 = diff --git a/src/openzl/common/operation_context.h b/src/openzl/common/operation_context.h index 2bccd90ef..079841a30 100644 --- a/src/openzl/common/operation_context.h +++ b/src/openzl/common/operation_context.h @@ -90,9 +90,8 @@ 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 The idx'th warning stored in the context. ZL_Error ZL_OC_getWarning(ZL_OperationContext const* opCtx, size_t idx); 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); From 69222ceb5bf6df8e656852a87004c201a069655e Mon Sep 17 00:00:00 2001 From: Felix Handte Date: Thu, 30 Apr 2026 10:15:39 -0700 Subject: [PATCH 3/4] Add Method to Find Error in Operation Context Summary: This is more general and simpler than `getErrorContextString()`, which has side-effects. This diff also implements `ZL_OC_getErrorContextString()` in terms of `ZL_OC_findError()`. Reviewed By: terrelln Differential Revision: D101645669 --- src/openzl/common/operation_context.c | 25 +++++++++++++++++-------- src/openzl/common/operation_context.h | 8 +++++++- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/openzl/common/operation_context.c b/src/openzl/common/operation_context.c index 2ea2b4a1d..f8b4bdf0a 100644 --- a/src/openzl/common/operation_context.c +++ b/src/openzl/common/operation_context.c @@ -151,7 +151,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) { @@ -159,18 +159,27 @@ 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 " diff --git a/src/openzl/common/operation_context.h b/src/openzl/common/operation_context.h index 079841a30..c9e1fd835 100644 --- a/src/openzl/common/operation_context.h +++ b/src/openzl/common/operation_context.h @@ -93,6 +93,12 @@ size_t ZL_OC_numWarnings(const ZL_OperationContext* opCtx); /// 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); @@ -101,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. From b3d78e2dd72e2b734eac51fba5314e4b13a05083 Mon Sep 17 00:00:00 2001 From: Felix Handte Date: Thu, 30 Apr 2026 10:15:39 -0700 Subject: [PATCH 4/4] C++: Add Method to Convert Exception Back to Error Summary: This will be useful when shoving C++ stuff into the OpenZL core. Differential Revision: D101645708 --- cpp/include/openzl/cpp/Exception.hpp | 57 ++++++++++++++++++++++++ cpp/tests/TestException.cpp | 51 +++++++++++++++++++++ include/openzl/detail/zl_error_context.h | 6 +++ include/openzl/detail/zl_errors_detail.h | 5 ++- src/openzl/common/operation_context.c | 8 ++++ 5 files changed, 125 insertions(+), 2 deletions(-) diff --git a/cpp/include/openzl/cpp/Exception.hpp b/cpp/include/openzl/cpp/Exception.hpp index 837567835..06aaeb851 100644 --- a/cpp/include/openzl/cpp/Exception.hpp +++ b/cpp/include/openzl/cpp/Exception.hpp @@ -46,6 +46,63 @@ class Exception : public std::runtime_error { } } + 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 { return errorContext_; 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 f8b4bdf0a..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 @@ -186,6 +187,13 @@ const char* ZL_OC_getErrorContextString( "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) {