simplify monitor socket RPC calls, fix missing error checks

The write-wait-read-check steps are always done together, so we might as
well merge them into one function call. Some callers were forgetting to
check for response errors and this change implicitly fixes them.

Test: launch CF
Change-Id: Ia77734743950f1ae5648c93441bcd728ceb03b34

9 files changed, 64 insertions, 101 deletions

diff --git a/host/commands/powerwash_cvd/powerwash_cvd.cc b/host/commands/powerwash_cvd/powerwash_cvd.cc
index d01c37f0e..8a535e332 100644
--- a/host/commands/powerwash_cvd/powerwash_cvd.cc
+++ b/host/commands/powerwash_cvd/powerwash_cvd.cc
@@ -48,14 +48,8 @@ Result<void> PowerwashCvdMain() {
       GetLauncherMonitor(*config, FLAGS_instance_num, FLAGS_wait_for_launcher));
 
   LOG(INFO) << "Requesting powerwash";
-  CF_EXPECT(WriteLauncherAction(monitor_socket, LauncherAction::kPowerwash));
-  CF_EXPECT(WaitForRead(monitor_socket, FLAGS_wait_for_launcher));
-  LauncherResponse powerwash_response =
-      CF_EXPECT(ReadLauncherResponse(monitor_socket));
-  CF_EXPECT(
-      powerwash_response == LauncherResponse::kSuccess,
-      "Received `" << static_cast<char>(powerwash_response)
-                   << "` response from launcher monitor for powerwash request");
+  CF_EXPECT(RunLauncherAction(monitor_socket, LauncherAction::kPowerwash,
+                              FLAGS_wait_for_launcher));
 
   LOG(INFO) << "Waiting for device to boot up again";
   CF_EXPECT(WaitForRead(monitor_socket, FLAGS_boot_timeout));
diff --git a/host/commands/record_cvd/record_cvd.cc b/host/commands/record_cvd/record_cvd.cc
index 33a9d1619..b41a0869f 100644
--- a/host/commands/record_cvd/record_cvd.cc
+++ b/host/commands/record_cvd/record_cvd.cc
@@ -56,20 +56,11 @@ Result<void> RecordCvdMain(int argc, char* argv[]) {
       CF_EXPECT(is_start ? SerializeStartScreenRecordingRequest()
                          : SerializeStopScreenRecordingRequest(),
                 "Failed to create serialized recording request proto.");
-  auto action_type = is_start ? ExtendedActionType::kStartScreenRecording
-                              : ExtendedActionType::kStopScreenRecording;
-  auto [serialized_data, extended_type] = RequestInfo{
-      .serialized_data = request, .extended_action_type = action_type};
-
-  CF_EXPECT(
-      WriteLauncherActionWithData(monitor_socket, LauncherAction::kExtended,
-                                  extended_type, std::move(serialized_data)));
-
-  LauncherResponse response = CF_EXPECT(ReadLauncherResponse(monitor_socket));
-  CF_EXPECTF(response == LauncherResponse::kSuccess,
-             "Received \"{}\" response from launcher monitor for \""
-             "{}\" request.",
-             static_cast<char>(response), command);
+  auto extended_action_type = is_start
+                                  ? ExtendedActionType::kStartScreenRecording
+                                  : ExtendedActionType::kStopScreenRecording;
+  CF_EXPECT(RunLauncherAction(monitor_socket, extended_action_type, request,
+                              std::nullopt));
   LOG(INFO) << "record_cvd " << command << " was successful.";
   return {};
 }
diff --git a/host/commands/restart_cvd/restart_cvd.cc b/host/commands/restart_cvd/restart_cvd.cc
index 44e81d1b7..3dac813bf 100644
--- a/host/commands/restart_cvd/restart_cvd.cc
+++ b/host/commands/restart_cvd/restart_cvd.cc
@@ -48,14 +48,8 @@ Result<void> RestartCvdMain() {
       GetLauncherMonitor(*config, FLAGS_instance_num, FLAGS_wait_for_launcher));
 
   LOG(INFO) << "Requesting restart";
-  CF_EXPECT(WriteLauncherAction(monitor_socket, LauncherAction::kRestart));
-  CF_EXPECT(WaitForRead(monitor_socket, FLAGS_wait_for_launcher));
-  LauncherResponse restart_response =
-      CF_EXPECT(ReadLauncherResponse(monitor_socket));
-  CF_EXPECT(
-      restart_response == LauncherResponse::kSuccess,
-      "Received `" << static_cast<char>(restart_response)
-                   << "` response from launcher monitor for restart request");
+  CF_EXPECT(RunLauncherAction(monitor_socket, LauncherAction::kRestart,
+                              FLAGS_wait_for_launcher));
 
   LOG(INFO) << "Waiting for device to boot up again";
   CF_EXPECT(WaitForRead(monitor_socket, FLAGS_boot_timeout));
diff --git a/host/commands/snapshot_util_cvd/main.cc b/host/commands/snapshot_util_cvd/main.cc
index 45f5afce6..f0051c99c 100644
--- a/host/commands/snapshot_util_cvd/main.cc
+++ b/host/commands/snapshot_util_cvd/main.cc
@@ -140,16 +140,8 @@ Result<void> SnapshotCvdMain(std::vector<std::string> args) {
 
     auto [serialized_data, extended_type] =
         CF_EXPECT(SerializeRequest(parsed.cmd, meta_json_path));
-    CF_EXPECT(
-        WriteLauncherActionWithData(monitor_socket, LauncherAction::kExtended,
-                                    extended_type, std::move(serialized_data)));
-    LOG(INFO) << "Wrote the extended serialized data and reading response";
-    LauncherResponse response = CF_EXPECT(ReadLauncherResponse(monitor_socket));
-    LOG(INFO) << "Read the response:  " << (int)LauncherResponse::kSuccess;
-    CF_EXPECTF(response == LauncherResponse::kSuccess,
-               "Received \"{}\" response from launcher monitor for \""
-               "{}\" request.",
-               static_cast<char>(response), static_cast<int>(parsed.cmd));
+    CF_EXPECT(RunLauncherAction(monitor_socket, extended_type,
+                                std::move(serialized_data), std::nullopt));
     LOG(INFO) << parsed.cmd << " was successful for instance #" << instance_num;
     if (parsed.cmd == SnapshotCmd::kSnapshotTake) {
       delete_snapshot_on_fail.Disable();
diff --git a/host/commands/status/main.cc b/host/commands/status/main.cc
index 8063b63ae..4e284f01d 100644
--- a/host/commands/status/main.cc
+++ b/host/commands/status/main.cc
@@ -129,14 +129,8 @@ Result<void> CvdStatusMain(const StatusFlags& flag_values) {
 
     LOG(INFO) << "Requesting status for instance "
               << instance_config.instance_name();
-    CF_EXPECT(WriteLauncherAction(monitor_socket, LauncherAction::kStatus));
-    CF_EXPECT(WaitForRead(monitor_socket, flag_values.wait_for_launcher));
-    LauncherResponse status_response =
-        CF_EXPECT(ReadLauncherResponse(monitor_socket));
-    CF_EXPECT(
-        status_response == LauncherResponse::kSuccess,
-        "Received `" << static_cast<char>(status_response)
-                     << "` response from launcher monitor for status request");
+    CF_EXPECT(RunLauncherAction(monitor_socket, LauncherAction::kStatus,
+                                flag_values.wait_for_launcher));
 
     devices_info[index] =
         PopulateDevicesInfoFromInstance(*config, instance_config);
diff --git a/host/commands/stop/main.cc b/host/commands/stop/main.cc
index 228c5c34b..7bab83194 100644
--- a/host/commands/stop/main.cc
+++ b/host/commands/stop/main.cc
@@ -122,14 +122,8 @@ Result<void> CleanStopInstance(
       GetLauncherMonitorFromInstance(instance_config, wait_for_launcher));
 
   LOG(INFO) << "Requesting stop";
-  CF_EXPECT(WriteLauncherAction(monitor_socket, LauncherAction::kStop));
-  CF_EXPECT(WaitForRead(monitor_socket, wait_for_launcher));
-  LauncherResponse stop_response =
-      CF_EXPECT(ReadLauncherResponse(monitor_socket));
-  CF_EXPECT(
-      stop_response == LauncherResponse::kSuccess,
-      "Received `" << static_cast<char>(stop_response)
-                   << "` response from launcher monitor for status request");
+  CF_EXPECT(RunLauncherAction(monitor_socket, LauncherAction::kStop,
+                              wait_for_launcher));
 
   LOG(INFO) << "Successfully stopped device " << instance_config.instance_name()
             << ": " << instance_config.adb_ip_and_port();
diff --git a/host/libs/command_util/util.cc b/host/libs/command_util/util.cc
index 3c4ae6652..0625134ec 100644
--- a/host/libs/command_util/util.cc
+++ b/host/libs/command_util/util.cc
@@ -32,6 +32,8 @@
 namespace cuttlefish {
 namespace {
 
+using run_cvd_msg_impl::LauncherActionMessage;
+
 template <typename T>
 Result<T> ReadFromMonitor(const SharedFD& monitor_socket) {
   T response;
@@ -46,10 +48,6 @@ Result<T> ReadFromMonitor(const SharedFD& monitor_socket) {
 
 }  // namespace
 
-Result<LauncherResponse> ReadLauncherResponse(const SharedFD& monitor_socket) {
-  return ReadFromMonitor<LauncherResponse>(monitor_socket);
-}
-
 Result<RunnerExitCodes> ReadExitCode(const SharedFD& monitor_socket) {
   return ReadFromMonitor<RunnerExitCodes>(monitor_socket);
 }
@@ -75,24 +73,6 @@ Result<SharedFD> GetLauncherMonitor(const CuttlefishConfig& config,
   return GetLauncherMonitorFromInstance(instance_config, timeout_seconds);
 }
 
-Result<void> WriteLauncherAction(const SharedFD& monitor_socket,
-                                 const LauncherAction request) {
-  CF_EXPECT(WriteLauncherActionWithData(monitor_socket, request,
-                                        ExtendedActionType::kUnused, ""));
-  return {};
-}
-
-Result<void> WriteLauncherActionWithData(const SharedFD& monitor_socket,
-                                         const LauncherAction request,
-                                         const ExtendedActionType type,
-                                         std::string serialized_data) {
-  using run_cvd_msg_impl::LauncherActionMessage;
-  auto message = CF_EXPECT(
-      LauncherActionMessage::Create(request, type, std::move(serialized_data)));
-  CF_EXPECT(message.WriteToFd(monitor_socket));
-  return {};
-}
-
 Result<LauncherActionInfo> ReadLauncherActionFromFd(
     const SharedFD& monitor_socket) {
   using run_cvd_msg_impl::LauncherActionMessage;
@@ -120,4 +100,36 @@ Result<void> WaitForRead(const SharedFD& monitor_socket,
   return {};
 }
 
+Result<void> RunLauncherAction(const SharedFD& monitor_socket,
+                               LauncherAction action,
+                               std::optional<int> timeout_seconds) {
+  auto message = CF_EXPECT(
+      LauncherActionMessage::Create(action, ExtendedActionType::kUnused, ""));
+  CF_EXPECT(message.WriteToFd(monitor_socket));
+  if (timeout_seconds.has_value()) {
+    CF_EXPECT(WaitForRead(monitor_socket, timeout_seconds.value()));
+  }
+  LauncherResponse response =
+      CF_EXPECT(ReadFromMonitor<LauncherResponse>(monitor_socket));
+  CF_EXPECT_EQ((int)response, (int)LauncherResponse::kSuccess);
+  return {};
+}
+
+Result<void> RunLauncherAction(const SharedFD& monitor_socket,
+                               ExtendedActionType extended_action_type,
+                               std::string serialized_data,
+                               std::optional<int> timeout_seconds) {
+  auto message = CF_EXPECT(LauncherActionMessage::Create(
+      LauncherAction::kExtended, extended_action_type,
+      std::move(serialized_data)));
+  CF_EXPECT(message.WriteToFd(monitor_socket));
+  if (timeout_seconds.has_value()) {
+    CF_EXPECT(WaitForRead(monitor_socket, timeout_seconds.value()));
+  }
+  LauncherResponse response =
+      CF_EXPECT(ReadFromMonitor<LauncherResponse>(monitor_socket));
+  CF_EXPECT_EQ((int)response, (int)LauncherResponse::kSuccess);
+  return {};
+}
+
 }  // namespace cuttlefish
diff --git a/host/libs/command_util/util.h b/host/libs/command_util/util.h
index ef9a41b1f..4ceeef271 100644
--- a/host/libs/command_util/util.h
+++ b/host/libs/command_util/util.h
@@ -23,8 +23,6 @@
 
 namespace cuttlefish {
 
-Result<LauncherResponse> ReadLauncherResponse(const SharedFD& monitor_socket);
-
 Result<RunnerExitCodes> ReadExitCode(const SharedFD& monitor_socket);
 
 Result<SharedFD> GetLauncherMonitorFromInstance(
@@ -35,21 +33,6 @@ Result<SharedFD> GetLauncherMonitor(const CuttlefishConfig& config,
                                     const int instance_num,
                                     const int timeout_seconds);
 
-Result<void> WriteLauncherAction(const SharedFD& monitor_socket,
-                                 const LauncherAction request);
-
-/**
- * Sends launcher actions with data
- *
- * If the request is something that does not use serialized_data at all,
- * the type should be ExtendedActionType::kUnused. serialized_data should
- * be std:move'd if avoiding redundant copy is desired.
- */
-Result<void> WriteLauncherActionWithData(const SharedFD& monitor_socket,
-                                         const LauncherAction request,
-                                         const ExtendedActionType type,
-                                         std::string serialized_data);
-
 struct LauncherActionInfo {
   LauncherAction action;
   ExtendedActionType type;
@@ -61,4 +44,17 @@ Result<LauncherActionInfo> ReadLauncherActionFromFd(
 Result<void> WaitForRead(const SharedFD& monitor_socket,
                          const int timeout_seconds);
 
+// Writes the `action` request to `monitor_socket`, then waits for the response
+// and checks for errors.
+Result<void> RunLauncherAction(const SharedFD& monitor_socket,
+                               LauncherAction action,
+                               std::optional<int> timeout_seconds);
+
+// Writes the extended action request serialized as `serialized_data` to
+// `monitor_socket`, then waits for the response and checks for errors.
+Result<void> RunLauncherAction(const SharedFD& monitor_socket,
+                               ExtendedActionType extended_action_type,
+                               std::string serialized_data,
+                               std::optional<int> timeout_seconds);
+
 }  // namespace cuttlefish
diff --git a/host/libs/process_monitor/process_monitor.cc b/host/libs/process_monitor/process_monitor.cc
index 4bd49408a..c06dd9da4 100644
--- a/host/libs/process_monitor/process_monitor.cc
+++ b/host/libs/process_monitor/process_monitor.cc
@@ -177,12 +177,8 @@ Result<void> SuspendResumeImpl(std::vector<MonitorEntry>& monitor_entries,
     auto serialized_request = CF_EXPECT(
         (is_suspend ? SerializeSuspendRequest() : SerializeResumeRequest()),
         "Failed to serialize request.");
-    CF_EXPECT(WriteLauncherActionWithData(
-        channel_to_secure_env, LauncherAction::kExtended, extended_type,
-        std::move(serialized_request)));
-    const std::string failed_command = (is_suspend ? "suspend" : "resume");
-    CF_EXPECT(ReadLauncherResponse(channel_to_secure_env),
-              "secure_env refused to " + failed_command);
+    CF_EXPECT(RunLauncherAction(channel_to_secure_env, extended_type,
+                                std::move(serialized_request), std::nullopt));
   }
 
   for (const auto& entry : monitor_entries) {