feat: Support disabling UDF stacktraces in Response

RomaService's Config object has been enhanced with a new field, disable_udf_stacktraces_in_response. When this field is enabled, the message on the absl::StatusOr<ResponseObject> will only contain the error, not the associated stacktrace. This flag does not affect logging functionality, UDF stacktraces will still be logged if a logging function is registered.

Bug: b/352016039
Change-Id: Ie4bcbf3c62802d0eefa129a800bd0d50c4943b4d
GitOrigin-RevId: 6b1139a051625f3b13680611da78059e7e5a3b96

src/roma/config/config.h

@@ -129,6 +129,11 @@ class Config {
    */
   bool enable_profilers = false;
 
+  /**
+   * @brief Disable returning UDF stack trace in response.
+   */
+  bool disable_udf_stacktraces_in_response = false;
+
   /**
    * @brief Indicates whether a custom logging function has been registered.
    *

src/roma/roma_service/BUILD.bazel

@@ -194,6 +194,7 @@ cc_test(
         "//src/roma/native_function_grpc_server:test_request_handlers",
         "//src/roma/native_function_grpc_server/proto:test_service_roma_host_api",
         "//src/util:duration",
+        "@com_google_absl//absl/log:scoped_mock_log",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/synchronization",
         "@com_google_absl//absl/time",

src/roma/roma_service/roma_service.h

@@ -306,7 +306,9 @@ class RomaService {
           config_.enable_sandbox_sharing_request_response_with_buffer_only,
           /*v8_flags=*/v8_flags,
           /*enable_profilers=*/config_.enable_profilers,
-          /*logging_function_set=*/config_.logging_function_set);
+          /*logging_function_set=*/config_.logging_function_set,
+          /*disable_udf_stacktraces_in_response*/
+          config_.disable_udf_stacktraces_in_response);
       PS_RETURN_IF_ERROR(workers_.back().Init());
       PS_RETURN_IF_ERROR(workers_.back().Run());
     }

src/roma/roma_service/sandboxed_service_test.cc

@@ -29,6 +29,8 @@
 
 #include <nlohmann/json.hpp>
 
+#include "absl/log/scoped_mock_log.h"
+#include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
 #include "absl/synchronization/mutex.h"
@@ -1936,6 +1938,136 @@ TEST(SandboxedServiceTest, ShouldGetExecutionErrorWhenJsCodeThrowError) {
   ASSERT_TRUE(roma_service.Stop().ok());
 }
 
+TEST(SandboxedServiceTest, StackTraceAvailableInResponse) {
+  Config config;
+  config.number_of_workers = 2;
+  RomaService<> roma_service(std::move(config));
+  ASSERT_TRUE(roma_service.Init().ok());
+
+  absl::Notification load_finished;
+  absl::Notification execute_failed;
+
+  {
+    auto code_obj = std::make_unique<CodeObject>(CodeObject{
+        .id = "foo",
+        .version_string = "v1",
+        .js = R"JS_CODE(
+      function Handler() {
+        throw new Error('foobar');
+      }
+    )JS_CODE",
+    });
+
+    absl::Status response_status;
+    ASSERT_TRUE(roma_service
+                    .LoadCodeObj(std::move(code_obj),
+                                 [&](absl::StatusOr<ResponseObject> resp) {
+                                   response_status = resp.status();
+                                   load_finished.Notify();
+                                 })
+                    .ok());
+    ASSERT_TRUE(
+        load_finished.WaitForNotificationWithTimeout(absl::Seconds(10)));
+    ASSERT_TRUE(response_status.ok());
+  }
+
+  {
+    auto execution_obj =
+        std::make_unique<InvocationStrRequest<>>(InvocationStrRequest<>{
+            .id = "foo",
+            .version_string = "v1",
+            .handler_name = "Handler",
+        });
+
+    absl::Status response_status;
+    ASSERT_TRUE(roma_service
+                    .Execute(std::move(execution_obj),
+                             [&](absl::StatusOr<ResponseObject> resp) {
+                               response_status = resp.status();
+                               execute_failed.Notify();
+                             })
+                    .ok());
+    ASSERT_TRUE(
+        execute_failed.WaitForNotificationWithTimeout(absl::Seconds(10)));
+    EXPECT_EQ(response_status.code(), absl::StatusCode::kInternal);
+    EXPECT_TRUE(absl::StrContains(response_status.message(), "at Handler"));
+  }
+
+  ASSERT_TRUE(roma_service.Stop().ok());
+}
+
+void LoggingFunction(absl::LogSeverity severity, DefaultMetadata metadata,
+                     std::string_view msg) {
+  LOG(LEVEL(severity)) << msg;
+}
+
+TEST(SandboxedServiceTest, ShouldNotGetStackTraceWhenDisableStackTraceFlagSet) {
+  Config config;
+  config.number_of_workers = 2;
+  config.disable_udf_stacktraces_in_response = true;
+  config.SetLoggingFunction(LoggingFunction);
+  RomaService<> roma_service(std::move(config));
+  ASSERT_TRUE(roma_service.Init().ok());
+
+  absl::Notification load_finished;
+  absl::Notification execute_failed;
+
+  absl::ScopedMockLog log;
+  // disable_udf_stacktraces_in_response should not prevent stacktraces from
+  // being logged.
+  EXPECT_CALL(log, Log(absl::LogSeverity::kError, _, HasSubstr("at Handler")));
+  log.StartCapturingLogs();
+
+  {
+    auto code_obj = std::make_unique<CodeObject>(CodeObject{
+        .id = "foo",
+        .version_string = "v1",
+        .js = R"JS_CODE(
+      function Handler() {
+        throw new Error('foobar');
+      }
+    )JS_CODE",
+    });
+
+    absl::Status response_status;
+    ASSERT_TRUE(roma_service
+                    .LoadCodeObj(std::move(code_obj),
+                                 [&](absl::StatusOr<ResponseObject> resp) {
+                                   response_status = resp.status();
+                                   load_finished.Notify();
+                                 })
+                    .ok());
+    ASSERT_TRUE(
+        load_finished.WaitForNotificationWithTimeout(absl::Seconds(10)));
+    ASSERT_TRUE(response_status.ok());
+  }
+
+  {
+    auto execution_obj =
+        std::make_unique<InvocationStrRequest<>>(InvocationStrRequest<>{
+            .id = "foo",
+            .version_string = "v1",
+            .handler_name = "Handler",
+        });
+
+    absl::Status response_status;
+    ASSERT_TRUE(roma_service
+                    .Execute(std::move(execution_obj),
+                             [&](absl::StatusOr<ResponseObject> resp) {
+                               response_status = resp.status();
+                               execute_failed.Notify();
+                             })
+                    .ok());
+    ASSERT_TRUE(
+        execute_failed.WaitForNotificationWithTimeout(absl::Seconds(10)));
+    EXPECT_EQ(response_status.code(), absl::StatusCode::kInternal);
+    EXPECT_FALSE(absl::StrContains(response_status.message(), "at Handler"));
+  }
+
+  ASSERT_TRUE(roma_service.Stop().ok());
+  log.StopCapturingLogs();
+}
+
 TEST(SandboxedServiceTest, ShouldGetExecutionErrorWhenJsCodeReturnUndefined) {
   Config config;
   config.number_of_workers = 2;

src/roma/sandbox/dispatcher/dispatcher_sapi_test.cc

@@ -55,7 +55,8 @@ std::vector<worker_api::WorkerSandboxApi> Workers(int num_workers) {
         /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
         /*v8_flags=*/std::vector<std::string>(),
         /*enable_profilers=*/false,
-        /*logging_function_set=*/false);
+        /*logging_function_set=*/false,
+        /*disable_udf_stacktraces_in_response=*/false);
     CHECK_OK(workers.back().Init());
     CHECK_OK(workers.back().Run());
   }

src/roma/sandbox/dispatcher/dispatcher_test.cc

@@ -56,7 +56,8 @@ std::vector<worker_api::WorkerSandboxApi> Workers(int num_workers) {
         /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
         /*v8_flags=*/std::vector<std::string>(),
         /*enable_profilers=*/false,
-        /*logging_function_set=*/false);
+        /*logging_function_set=*/false,
+        /*disable_udf_stacktraces_in_response=*/false);
     CHECK_OK(workers.back().Init());
     CHECK_OK(workers.back().Run());
   }

src/roma/sandbox/js_engine/v8_engine/v8_js_engine.cc

@@ -210,13 +210,16 @@ V8JsEngine::V8JsEngine(
     std::unique_ptr<V8IsolateFunctionBinding> isolate_function_binding,
     const bool skip_v8_cleanup, const bool enable_profilers,
     const JsEngineResourceConstraints& v8_resource_constraints,
-    const bool logging_function_set)
+    const bool logging_function_set,
+    const bool disable_udf_stacktraces_in_response)
     : isolate_function_binding_(std::move(isolate_function_binding)),
       v8_resource_constraints_(v8_resource_constraints),
       execution_watchdog_(std::make_unique<roma::worker::ExecutionWatchDog>()),
       skip_v8_cleanup_(skip_v8_cleanup),
       enable_profilers_(enable_profilers),
-      logging_function_set_(logging_function_set) {
+      logging_function_set_(logging_function_set),
+      disable_udf_stacktraces_in_response_(
+          disable_udf_stacktraces_in_response) {
   if (isolate_function_binding_) {
     isolate_function_binding_->AddExternalReferences(external_references_);
   }
@@ -287,11 +290,15 @@ absl::Status V8JsEngine::FormatAndLogError(v8::Isolate* isolate,
                                            v8::Local<v8::Context> context,
                                            std::string_view top_level_error,
                                            LogOptions log_options) {
-  std::string err_msg =
-      absl::StrCat(GetError(isolate, try_catch, top_level_error), "\n",
-                   GetStackTrace(isolate, try_catch, context));
-  HandleLog("ROMA_ERROR", err_msg, std::move(log_options)).IgnoreError();
-  return absl::InternalError(std::move(err_msg));
+  std::string err_msg = GetError(isolate, try_catch, top_level_error);
+  std::string stack_trace = GetStackTrace(isolate, try_catch, context);
+  std::string err_msg_stack_trace = absl::StrCat(err_msg, "\n", stack_trace);
+  HandleLog("ROMA_ERROR", err_msg_stack_trace, std::move(log_options))
+      .IgnoreError();
+  if (disable_udf_stacktraces_in_response_) {
+    return absl::InternalError(std::move(err_msg));
+  }
+  return absl::InternalError(std::move(err_msg_stack_trace));
 }
 
 absl::Status V8JsEngine::HandleLog(std::string_view function_name,

src/roma/sandbox/js_engine/v8_engine/v8_js_engine.h

@@ -49,7 +49,8 @@ class V8JsEngine : public JsEngine {
              bool skip_v8_cleanup = false, bool enable_profilers = false,
              const JsEngineResourceConstraints& v8_resource_constraints =
                  JsEngineResourceConstraints(),
-             bool logging_function_set = false);
+             bool logging_function_set = false,
+             bool disable_udf_stacktraces_in_response = false);
 
   ~V8JsEngine() override;
 
@@ -230,6 +231,7 @@ class V8JsEngine : public JsEngine {
   const bool skip_v8_cleanup_;
   const bool enable_profilers_;
   const bool logging_function_set_;
+  const bool disable_udf_stacktraces_in_response_;
 };
 }  // namespace google::scp::roma::sandbox::js_engine::v8_js_engine
 

src/roma/sandbox/worker_api/sapi/utils.cc

@@ -64,7 +64,7 @@ std::unique_ptr<Worker> CreateWorker(const V8WorkerEngineParams& params) {
   auto v8_engine = std::make_unique<V8JsEngine>(
       std::move(isolate_function_binding), params.skip_v8_cleanup,
       params.enable_profilers, params.resource_constraints,
-      params.logging_function_set);
+      params.logging_function_set, params.disable_udf_stacktraces_in_response);
   v8_engine->OneTimeSetup(GetEngineOneTimeSetup(params));
   return std::make_unique<Worker>(std::move(v8_engine), params.require_preload);
 }

src/roma/sandbox/worker_api/sapi/utils.h

@@ -44,6 +44,7 @@ struct V8WorkerEngineParams {
   std::vector<std::string> v8_flags;
   bool enable_profilers = false;
   bool logging_function_set = false;
+  bool disable_udf_stacktraces_in_response = false;
 };
 
 absl::flat_hash_map<std::string, std::string> GetEngineOneTimeSetup(

src/roma/sandbox/worker_api/sapi/worker_init_params.proto

@@ -63,4 +63,7 @@ message WorkerInitParamsProto {
 
   // Set if a custom logging function was set on the Config passed into Roma.
   bool logging_function_set = 16;
+
+  // Whether UDF stacktraces should be included in the response.
+  bool disable_udf_stacktraces_in_response = 17;
 }

src/roma/sandbox/worker_api/sapi/worker_sandbox_api.cc

@@ -55,7 +55,7 @@ WorkerSandboxApi::WorkerSandboxApi(
     size_t sandbox_request_response_shared_buffer_size_mb,
     bool enable_sandbox_sharing_request_response_with_buffer_only,
     const std::vector<std::string>& v8_flags, bool enable_profilers,
-    bool logging_function_set)
+    bool logging_function_set, bool disable_udf_stacktraces_in_response)
     : require_preload_(require_preload),
       native_js_function_comms_fd_(native_js_function_comms_fd),
       native_js_function_names_(native_js_function_names),
@@ -70,7 +70,9 @@ WorkerSandboxApi::WorkerSandboxApi(
           enable_sandbox_sharing_request_response_with_buffer_only),
       v8_flags_(v8_flags),
       enable_profilers_(enable_profilers),
-      logging_function_set_(logging_function_set) {
+      logging_function_set_(logging_function_set),
+      disable_udf_stacktraces_in_response_(
+          disable_udf_stacktraces_in_response) {
   // create a sandbox2 buffer
   request_and_response_data_buffer_size_bytes_ =
       sandbox_request_response_shared_buffer_size_mb > 0
@@ -173,6 +175,8 @@ absl::Status WorkerSandboxApi::Init() {
                                                 v8_flags_.end());
   worker_init_params.set_enable_profilers(enable_profilers_);
   worker_init_params.set_logging_function_set(logging_function_set_);
+  worker_init_params.set_disable_udf_stacktraces_in_response(
+      disable_udf_stacktraces_in_response_);
 
   const auto worker_status = worker_wrapper_->Init(worker_init_params);
   if (!worker_status.ok()) {

src/roma/sandbox/worker_api/sapi/worker_sandbox_api.h

@@ -66,6 +66,10 @@ class WorkerSandboxApi {
    * @param v8_flags List of flags to pass into v8. (Ex. {"--FLAG_1",
    * "--FLAG_2"})
    * @param enable_profilers Enable the V8 CPU and Heap Profilers
+   * @param logging_function_set Whether a logging function has been registered
+   * in Roma
+   * @param disable_udf_stacktraces_in_response Whether UDF stacktrace should be
+   * returned in response
    */
   WorkerSandboxApi(
       bool require_preload, int native_js_function_comms_fd,
@@ -78,7 +82,7 @@ class WorkerSandboxApi {
       size_t sandbox_request_response_shared_buffer_size_mb,
       bool enable_sandbox_sharing_request_response_with_buffer_only,
       const std::vector<std::string>& v8_flags, bool enable_profilers,
-      bool logging_function_set);
+      bool logging_function_set, bool disable_udf_stacktraces_in_response);
 
   absl::Status Init();
 
@@ -136,6 +140,7 @@ class WorkerSandboxApi {
   std::vector<std::string> v8_flags_;
   const bool enable_profilers_;
   const bool logging_function_set_;
+  const bool disable_udf_stacktraces_in_response_;
 };
 }  // namespace google::scp::roma::sandbox::worker_api
 

src/roma/sandbox/worker_api/sapi/worker_sandbox_api_sapi_test.cc

@@ -56,7 +56,8 @@ TEST(WorkerSandboxApiSapiTest,
       /*sandbox_request_response_shared_buffer_size_mb=*/0,
       /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
       /*v8_flags=*/{}, /*enable_profilers=*/false,
-      /*logging_function_set=*/false);
+      /*logging_function_set=*/false,
+      /*disable_udf_stacktraces_in_response=*/false);
 
   // Initializing the sandbox fail as we're giving a max of 100MB of virtual
   // space address for v8 and the sandbox.
@@ -81,7 +82,8 @@ class WorkerSandboxApiForTests : public WorkerSandboxApi {
             /*sandbox_request_response_shared_buffer_size_mb=*/0,
             /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
             /*v8_flags=*/{}, /*enable_profilers=*/false,
-            /*logging_function_set=*/false) {}
+            /*logging_function_set=*/false,
+            /*disable_udf_stacktraces_in_response=*/false) {}
 
   ::sapi::Sandbox* GetUnderlyingSandbox() { return worker_sapi_sandbox_.get(); }
 };

src/roma/sandbox/worker_api/sapi/worker_sandbox_api_test.cc

@@ -54,7 +54,8 @@ TEST(WorkerSandboxApiTest, WorkerWorksThroughSandbox) {
       /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
       /*v8_flags=*/{},
       /*enable_profilers=*/false,
-      /*logging_function_set=*/false);
+      /*logging_function_set=*/false,
+      /*disable_udf_stacktraces_in_response=*/false);
 
   ASSERT_TRUE(sandbox_api.Init().ok());
 
@@ -89,7 +90,8 @@ TEST(WorkerSandboxApiTest, WorkerReturnsInformativeThrowMessageThroughSandbox) {
       /*sandbox_request_response_shared_buffer_size_mb=*/0,
       /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
       /*v8_flags=*/{}, /*enable_profilers=*/false,
-      /*logging_function_set=*/false);
+      /*logging_function_set=*/false,
+      /*disable_udf_stacktraces_in_response=*/false);
 
   ASSERT_TRUE(sandbox_api.Init().ok());
 
@@ -123,7 +125,8 @@ TEST(WorkerSandboxApiTest, WorkerReturnsInformativeMessageForMissingParam) {
       /*sandbox_request_response_shared_buffer_size_mb=*/0,
       /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
       /*v8_flags=*/{}, /*enable_cpu_profiler=*/false,
-      /*logging_function_set=*/false);
+      /*logging_function_set=*/false,
+      /*disable_udf_stacktraces_in_response=*/false);
 
   ASSERT_TRUE(sandbox_api.Init().ok());
 
@@ -159,7 +162,8 @@ TEST(WorkerSandboxApiTest, WorkerCanCallHooksThroughSandbox) {
       /*sandbox_request_response_shared_buffer_size_mb=*/0,
       /*enable_sandbox_sharing_request_response_with_buffer_only=*/false,
       /*v8_flags=*/{}, /*enable_profilers=*/false,
-      /*logging_function_set=*/false);
+      /*logging_function_set=*/false,
+      /*disable_udf_stacktraces_in_response=*/false);
 
   ASSERT_TRUE(sandbox_api.Init().ok());