Combine source location with throw-site stack trace

Summary: Combines source location and throw-site stack trace in the builder constructor.

Reviewed By: vilatto

Differential Revision: D99200513

fbshipit-source-id: 22eb699427d3fc0b17de3f9406c37329f08e6fac

Author: Chandhana Solainathan

eden/fs/telemetry/EdenErrorInfoBuilder.cpp

@@ -50,8 +50,6 @@ EdenErrorInfo EdenErrorInfoBuilder::create() {
   info.errorCode = errorCode_;
   info.errorName = std::move(errorName_);
   info.exceptionType = std::move(exceptionType_);
-  // Currently stores source location (file:line in func), will be replaced
-  // with a Manifold URL for full stack traces
   info.stackTrace = std::move(sourceLocation_);
   info.clientCommandName = std::move(clientCommandName_);
   info.inode = inode_;
@@ -70,6 +68,17 @@ EdenErrorInfoBuilder::EdenErrorInfoBuilder(
       errorName_(error.errorName),
       exceptionType_(error.exceptionType),
       sourceLocation_(
-          fmt::format("{}:{} in {}", loc.file, loc.line, loc.func)) {}
+          error.stackTrace.has_value()
+              ? fmt::format(
+                    "Source: {}:{} in {}\n\nStack trace:\n{}",
+                    loc.file,
+                    loc.line,
+                    loc.func,
+                    *error.stackTrace)
+              : fmt::format(
+                    "Source: {}:{} in {}",
+                    loc.file,
+                    loc.line,
+                    loc.func)) {}
 
 } // namespace facebook::eden

eden/fs/telemetry/test/BUCK

@@ -96,6 +96,8 @@ cpp_unittest(
     deps = [
         "fbsource//third-party/googletest:gtest",
         "//eden/fs/telemetry:error_info",
+        "//eden/fs/telemetry:throw_trace_capture",  # @manual needed for --wrap __cxa_throw linker flag
+        "//folly:c_portability",
     ],
 )
 

eden/fs/telemetry/test/EdenErrorInfoBuilderTest.cpp

@@ -8,27 +8,56 @@
 #include "eden/fs/telemetry/EdenErrorInfoBuilder.h"
 #include <gtest/gtest.h>
 #include <cerrno>
+#include <stdexcept>
 #include <system_error>
 
+#include <folly/CPortability.h>
+
+#include "eden/fs/telemetry/ErrorArg.h"
+
 using namespace facebook::eden;
 
+namespace {
+[[noreturn]] FOLLY_NOINLINE void throwRuntimeError() {
+  throw std::runtime_error("fuse read failed");
+}
+} // namespace
+
 TEST(EdenErrorInfoTest, InitializeFuseEdenErrorInfoWithException) {
-  std::runtime_error ex("fuse read failed");
-  int line = __LINE__ + 1;
-  auto info = EdenErrorInfo::fuse(ex, 42, "/mnt/repo").create();
+  try {
+    throwRuntimeError();
+  } catch (const std::exception& ex) {
+    // ErrorArg captures the throw-site stack trace inside a catch block.
+    ErrorArg error(ex);
+#ifdef __linux__
+    ASSERT_TRUE(error.stackTrace.has_value())
+        << "ErrorArg should capture a stack trace from a thrown exception";
+    EXPECT_NE(error.stackTrace->find("throwRuntimeError"), std::string::npos)
+        << "Stack trace should contain the throwing function, got: "
+        << *error.stackTrace;
+#endif
 
-  EXPECT_EQ(info.component, EdenComponent::Fuse);
-  EXPECT_EQ(info.errorMessage, "fuse read failed");
-  EXPECT_EQ(info.inode.value(), 42);
-  EXPECT_EQ(info.mountPoint.value(), "/mnt/repo");
-  EXPECT_FALSE(info.errorCode.has_value());
-  EXPECT_NE(
-      info.exceptionType.value().find("runtime_error"), std::string::npos);
-  EXPECT_TRUE(info.stackTrace.has_value());
-  auto& trace = info.stackTrace.value();
-  EXPECT_NE(trace.find("EdenErrorInfoBuilderTest.cpp"), std::string::npos);
-  EXPECT_NE(trace.find(":" + std::to_string(line) + " in "), std::string::npos);
-  EXPECT_NE(trace.find("TestBody"), std::string::npos);
+    // create() stores the raw combined trace in info.stackTrace.
+    auto info = EdenErrorInfo::fuse(ex, 42, "/mnt/repo").create();
+
+    EXPECT_EQ(info.component, EdenComponent::Fuse);
+    EXPECT_EQ(info.errorMessage, "fuse read failed");
+    EXPECT_EQ(info.inode.value(), 42);
+    EXPECT_EQ(info.mountPoint.value(), "/mnt/repo");
+    EXPECT_FALSE(info.errorCode.has_value());
+    EXPECT_NE(
+        info.exceptionType.value().find("runtime_error"), std::string::npos);
+    ASSERT_TRUE(info.stackTrace.has_value());
+    EXPECT_NE(
+        info.stackTrace->find("EdenErrorInfoBuilderTest.cpp"),
+        std::string::npos)
+        << "Stack trace should contain source file, got: " << *info.stackTrace;
+#ifdef __linux__
+    EXPECT_NE(info.stackTrace->find("Stack trace:"), std::string::npos)
+        << "Stack trace should contain raw trace section, got: "
+        << *info.stackTrace;
+#endif
+  }
 }
 
 TEST(EdenErrorInfoTest, InitializeFuseEdenErrorInfoWithStringMessage) {
@@ -41,10 +70,10 @@ TEST(EdenErrorInfoTest, InitializeFuseEdenErrorInfoWithStringMessage) {
   EXPECT_FALSE(info.errorCode.has_value());
   EXPECT_FALSE(info.errorName.has_value());
   EXPECT_FALSE(info.exceptionType.has_value());
-  EXPECT_TRUE(info.stackTrace.has_value());
-  auto& loc = info.stackTrace.value();
-  EXPECT_NE(loc.find("EdenErrorInfoBuilderTest.cpp"), std::string::npos)
-      << "Expected source file in location, got: " << loc;
+  ASSERT_TRUE(info.stackTrace.has_value());
+  EXPECT_NE(
+      info.stackTrace->find("EdenErrorInfoBuilderTest.cpp"), std::string::npos)
+      << "Stack trace should contain source file, got: " << *info.stackTrace;
 }
 
 TEST(EdenErrorInfoTest, FuseErrorInfoOverridesErrorCodeAndName) {