YT-21233: Remove TSimpleException and teach TCompositeException storing simple attributes

Expand the CompositeException
9a10ec65bfc1df854e03bb3a4d8d0a0c0e4a3a5d

11 files changed, 200 insertions, 44 deletions

diff --git a/library/cpp/yt/exception/attributes.h b/library/cpp/yt/exception/attributes.h
new file mode 100644
index 0000000000..c1a88bb820
--- /dev/null
+++ b/library/cpp/yt/exception/attributes.h
@@ -0,0 +1,30 @@
+#pragma once
+
+#include <library/cpp/yt/misc/guid.h>
+
+#include <util/generic/string.h>
+
+#include <variant>
+
+namespace NYT {
+
+////////////////////////////////////////////////////////////////////////////////
+
+struct TExceptionAttribute
+{
+    using TKey = TString;
+    using TValue = std::variant<i64, double, bool, TString>;
+
+    template <class T>
+    TExceptionAttribute(const TKey& key, const T& value)
+        : Key(key)
+        , Value(value)
+    { }
+
+    TKey Key;
+    TValue Value;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT
diff --git a/library/cpp/yt/exception/exception.cpp b/library/cpp/yt/exception/exception.cpp
index 2729511d5a..16457582ec 100644
--- a/library/cpp/yt/exception/exception.cpp
+++ b/library/cpp/yt/exception/exception.cpp
@@ -1,46 +1,94 @@
 #include "exception.h"
 
+#include <library/cpp/yt/assert/assert.h>
+
 namespace NYT {
 
 ////////////////////////////////////////////////////////////////////////////////
 
+template <class TRange>
+    // requires (!std::is_lvalue_reference_v<TRange>)
+void AddAttributes(TSimpleException::TAttributes& attrs, TRange&& range)
+{
+    for (auto&& [key, value] : range) {
+        YT_VERIFY(attrs.emplace(std::move(key), std::move(value)).second);
+    }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
 TSimpleException::TSimpleException(TString message)
     : Message_(std::move(message))
+    , What_(Message_)
+{ }
+
+TSimpleException::TSimpleException(
+    const std::exception& exception,
+    TString message)
+    : InnerException_(std::current_exception())
+    , Message_(std::move(message))
+    , What_(Message_ + "\n" + exception.what())
 { }
 
+const std::exception_ptr& TSimpleException::GetInnerException() const
+{
+    return InnerException_;
+}
+
+const char* TSimpleException::what() const noexcept
+{
+    return What_.c_str();
+}
+
 const TString& TSimpleException::GetMessage() const
 {
     return Message_;
 }
 
-const char* TSimpleException::what() const noexcept
+const TSimpleException::TAttributes& TSimpleException::GetAttributes() const &
 {
-    return Message_.c_str();
+    return Attributes_;
 }
 
-////////////////////////////////////////////////////////////////////////////////
+TSimpleException::TAttributes&& TSimpleException::GetAttributes() &&
+{
+    return std::move(Attributes_);
+}
 
-TCompositeException::TCompositeException(TString message)
-    : TSimpleException(std::move(message))
-    , What_(Message_)
-{ }
+TSimpleException& TSimpleException::operator<<= (TExceptionAttribute&& attribute) &
+{
+    YT_VERIFY(Attributes_.emplace(std::move(attribute.Key), std::move(attribute.Value)).second);
+    return *this;
+}
 
-TCompositeException::TCompositeException(
-    const std::exception& exception,
-    TString message)
-    : TSimpleException(message)
-    , InnerException_(std::current_exception())
-    , What_(message + "\n" + exception.what())
-{ }
+TSimpleException& TSimpleException::operator<<= (std::vector<TExceptionAttribute>&& attributes) &
+{
+    AddAttributes(Attributes_, std::move(attributes));
+    return *this;
+}
 
-const std::exception_ptr& TCompositeException::GetInnerException() const
+TSimpleException& TSimpleException::operator<<= (TAttributes&& attributes) &
 {
-    return InnerException_;
+    AddAttributes(Attributes_, std::move(attributes));
+    return *this;
 }
 
-const char* TCompositeException::what() const noexcept
+TSimpleException& TSimpleException::operator<<= (const TExceptionAttribute& attribute) &
 {
-    return What_.c_str();
+    YT_VERIFY(Attributes_.emplace(attribute.Key, attribute.Value).second);
+    return *this;
+}
+
+TSimpleException& TSimpleException::operator<<= (const std::vector<TExceptionAttribute>& attributes) &
+{
+    AddAttributes(Attributes_, attributes);
+    return *this;
+}
+
+TSimpleException& TSimpleException::operator<<= (const TAttributes& attributes) &
+{
+    AddAttributes(Attributes_, attributes);
+    return *this;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/library/cpp/yt/exception/exception.h b/library/cpp/yt/exception/exception.h
index 1ac3a76ea4..2898b99f0d 100644
--- a/library/cpp/yt/exception/exception.h
+++ b/library/cpp/yt/exception/exception.h
@@ -1,43 +1,74 @@
 #pragma once
 
-#include <util/generic/string.h>
+#include "attributes.h"
+
+#include <util/generic/hash.h>
 
 #include <exception>
 
 namespace NYT {
 
 ////////////////////////////////////////////////////////////////////////////////
-// These are poor man's versions of NYT::TErrorException to be used in
+// This is poor man's version of NYT::TErrorException to be used in
 // a limited subset of core libraries that are needed to implement NYT::TError.
 
 class TSimpleException
     : public std::exception
 {
 public:
-    explicit TSimpleException(TString message);
-
-    const TString& GetMessage() const;
-    const char* what() const noexcept override;
+    using TAttributes = THashMap<
+        TExceptionAttribute::TKey,
+        TExceptionAttribute::TValue>;
 
-protected:
-    const TString Message_;
-};
+    template <class TValue>
+    static constexpr bool CNestable = requires (TSimpleException& ex, TValue&& operand) {
+        { ex <<= std::forward<TValue>(operand) } -> std::same_as<TSimpleException&>;
+    };
 
-class TCompositeException
-    : public TSimpleException
-{
-public:
-    explicit TCompositeException(TString message);
-    TCompositeException(
+    explicit TSimpleException(TString message);
+    TSimpleException(
         const std::exception& exception,
         TString message);
 
     const std::exception_ptr& GetInnerException() const;
     const char* what() const noexcept override;
 
+    const TString& GetMessage() const;
+
+    const TAttributes& GetAttributes() const &;
+    TAttributes&& GetAttributes() &&;
+
+    TSimpleException& operator<<= (TExceptionAttribute&& attribute) &;
+    TSimpleException& operator<<= (std::vector<TExceptionAttribute>&& attributes) &;
+    TSimpleException& operator<<= (TAttributes&& attributes) &;
+
+    TSimpleException& operator<<= (const TExceptionAttribute& attribute) &;
+    TSimpleException& operator<<= (const std::vector<TExceptionAttribute>& attributes) &;
+    TSimpleException& operator<<= (const TAttributes& attributes) &;
+
+    // NB: clang is incapable of parsing such requirements (which refer back to the class) out-of-line.
+    // To keep this overload from winning in resolution
+    // when constraint actually fails, we define method right here.
+    template <class TValue>
+        requires CNestable<TValue>
+    TSimpleException&& operator<< (TValue&& value) &&
+    {
+        return std::move(*this <<= std::forward<TValue>(value));
+    }
+
+    template <class TValue>
+        requires CNestable<TValue>
+    TSimpleException operator<< (TValue&& value) const &
+    {
+        return TSimpleException(*this) << std::forward<TValue>(value);
+    }
+
 private:
     const std::exception_ptr InnerException_;
+    const TString Message_;
     const TString What_;
+
+    TAttributes Attributes_;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/library/cpp/yt/exception/ya.make b/library/cpp/yt/exception/ya.make
index ff0014adcf..954a9aca6e 100644
--- a/library/cpp/yt/exception/ya.make
+++ b/library/cpp/yt/exception/ya.make
@@ -6,4 +6,8 @@ SRCS(
     exception.cpp
 )
 
+PEERDIR(
+    library/cpp/yt/assert
+)
+
 END()
diff --git a/library/cpp/yt/misc/guid.cpp b/library/cpp/yt/misc/guid.cpp
index 02c396b015..fc4f146532 100644
--- a/library/cpp/yt/misc/guid.cpp
+++ b/library/cpp/yt/misc/guid.cpp
@@ -1,5 +1,7 @@
 #include "guid.h"
 
+#include <library/cpp/yt/exception/exception.h>
+
 #include <util/random/random.h>
 
 #include <util/string/printf.h>
diff --git a/library/cpp/yt/misc/guid.h b/library/cpp/yt/misc/guid.h
index 05844ba49c..48912b105f 100644
--- a/library/cpp/yt/misc/guid.h
+++ b/library/cpp/yt/misc/guid.h
@@ -3,8 +3,6 @@
 #include <util/generic/string.h>
 #include <util/generic/typetraits.h>
 
-#include <library/cpp/yt/exception/exception.h>
-
 #include <array>
 
 namespace NYT {
diff --git a/library/cpp/yt/string/string.cpp b/library/cpp/yt/string/string.cpp
index a96f212cdf..c4bd98b66c 100644
--- a/library/cpp/yt/string/string.cpp
+++ b/library/cpp/yt/string/string.cpp
@@ -3,6 +3,8 @@
 
 #include <library/cpp/yt/assert/assert.h>
 
+#include <library/cpp/yt/exception/exception.h>
+
 #include <util/generic/hash.h>
 
 #include <util/string/ascii.h>
diff --git a/library/cpp/yt/string/string.h b/library/cpp/yt/string/string.h
index 04eb115cb2..a131b40a2a 100644
--- a/library/cpp/yt/string/string.h
+++ b/library/cpp/yt/string/string.h
@@ -2,8 +2,6 @@
 
 #include "string_builder.h"
 
-#include <library/cpp/yt/exception/exception.h>
-
 #include <util/datetime/base.h>
 
 #include <util/generic/string.h>
diff --git a/library/cpp/yt/yson_string/convert.h b/library/cpp/yt/yson_string/convert.h
index 3c2cc7d284..06de28d2f9 100644
--- a/library/cpp/yt/yson_string/convert.h
+++ b/library/cpp/yt/yson_string/convert.h
@@ -69,10 +69,10 @@ TYsonString ConvertToYsonString<TGuid>(const TGuid& value);
 // Note: these currently support a subset of NYT::NYTree::Convert features.
 
 class TYsonLiteralParseException
-    : public TCompositeException
+    : public TSimpleException
 {
 public:
-    using TCompositeException::TCompositeException;
+    using TSimpleException::TSimpleException;
 };
 
 template <>
diff --git a/yt/yt/core/misc/error.cpp b/yt/yt/core/misc/error.cpp
index bbc950b87f..cc6ae70bf9 100644
--- a/yt/yt/core/misc/error.cpp
+++ b/yt/yt/core/misc/error.cpp
@@ -372,12 +372,24 @@ TError::TErrorOr(TError&& other) noexcept
 
 TError::TErrorOr(const std::exception& ex)
 {
-    if (const auto* compositeException = dynamic_cast<const TCompositeException*>(&ex)) {
+    if (auto simpleException = dynamic_cast<const TSimpleException*>(&ex)) {
+        *this = TError(NYT::EErrorCode::Generic, simpleException->GetMessage());
+        // NB: clang-14 is incapable of capturing structured binding variables
+        //  so we force materialize them via this function call.
+        auto addAttribute = [this] (const auto& key, const auto& value) {
+            std::visit([&] (const auto& actual) {
+                *this <<= TErrorAttribute(key, actual);
+            }, value);
+        };
+        for (const auto& [key, value] : simpleException->GetAttributes()) {
+            addAttribute(key, value);
+        }
         try {
-            std::rethrow_exception(compositeException->GetInnerException());
+            if (simpleException->GetInnerException()) {
+                std::rethrow_exception(simpleException->GetInnerException());
+            }
         } catch (const std::exception& innerEx) {
-            *this = TError(NYT::EErrorCode::Generic, compositeException->GetMessage())
-                << TError(innerEx);
+            *this <<= TError(innerEx);
         }
     } else if (const auto* errorEx = dynamic_cast<const TErrorException*>(&ex)) {
         *this = errorEx->Error();
diff --git a/yt/yt/core/misc/unittests/error_ut.cpp b/yt/yt/core/misc/unittests/error_ut.cpp
index e83e762358..f1fc1d9bd4 100644
--- a/yt/yt/core/misc/unittests/error_ut.cpp
+++ b/yt/yt/core/misc/unittests/error_ut.cpp
@@ -670,7 +670,7 @@ TEST(TErrorTest, CompositeYTExceptionToError)
         try {
             throw TSimpleException("inner message");
         } catch (const std::exception& ex) {
-            throw TCompositeException(ex, "outer message");
+            throw TSimpleException(ex, "outer message");
         }
     } catch (const std::exception& ex) {
         TError outerError(ex);
@@ -683,6 +683,37 @@ TEST(TErrorTest, CompositeYTExceptionToError)
     }
 }
 
+TEST(TErrorTest, YTExceptionWithAttributesToError)
+{
+    try {
+        throw TSimpleException("message")
+            << TExceptionAttribute("Int64 value", static_cast<i64>(42))
+            << TExceptionAttribute("double value", 7.77)
+            << TExceptionAttribute("bool value", false)
+            << TExceptionAttribute("String value", "FooBar");
+    } catch (const std::exception& ex) {
+        TError error(ex);
+        EXPECT_EQ(NYT::EErrorCode::Generic, error.GetCode());
+        EXPECT_EQ("message", error.GetMessage());
+
+        auto i64value = error.Attributes().Find<i64>("Int64 value");
+        EXPECT_TRUE(i64value);
+        EXPECT_EQ(*i64value, static_cast<i64>(42));
+
+        auto doubleValue = error.Attributes().Find<double>("double value");
+        EXPECT_TRUE(doubleValue);
+        EXPECT_EQ(*doubleValue, 7.77);
+
+        auto boolValue = error.Attributes().Find<bool>("bool value");
+        EXPECT_TRUE(boolValue);
+        EXPECT_EQ(*boolValue, false);
+
+        auto stringValue = error.Attributes().Find<TString>("String value");
+        EXPECT_TRUE(stringValue);
+        EXPECT_EQ(*stringValue, "FooBar");
+    }
+}
+
 TEST(TErrorTest, ErrorSanitizer)
 {
     auto checkSantizied = [&] (const TError& error) {