analytics: Add operation result code for Payload analytics, IV

PiperOrigin-RevId: 704265442

connections/implementation/internal_payload_factory.cc

@@ -27,6 +27,7 @@
 #include "connections/payload_type.h"
 #include "internal/platform/byte_array.h"
 #include "internal/platform/exception.h"
+#include "internal/platform/expected.h"
 #include "internal/platform/file.h"
 #include "internal/platform/implementation/platform.h"
 #include "internal/platform/input_stream.h"
@@ -40,6 +41,7 @@ namespace connections {
 
 namespace {
 using ::location::nearby::connections::PayloadTransferFrame;
+using ::location::nearby::proto::connections::OperationResultCode;
 
 class BytesInternalPayload : public InternalPayload {
  public:
@@ -309,23 +311,24 @@ class IncomingFileInternalPayload : public InternalPayload {
 using ::nearby::api::ImplementationPlatform;
 using ::nearby::api::OSName;
 
-std::unique_ptr<InternalPayload> CreateOutgoingInternalPayload(
+ErrorOr<std::unique_ptr<InternalPayload>> CreateOutgoingInternalPayload(
     Payload payload) {
   switch (payload.GetType()) {
     case PayloadType::kBytes:
-      return std::make_unique<BytesInternalPayload>(std::move(payload));
+      return {std::make_unique<BytesInternalPayload>(std::move(payload))};
 
     case PayloadType::kFile: {
-      return std::make_unique<OutgoingFileInternalPayload>(std::move(payload));
+      return {
+          std::make_unique<OutgoingFileInternalPayload>(std::move(payload))};
     }
 
     case PayloadType::kStream:
-      return std::make_unique<OutgoingStreamInternalPayload>(
-          std::move(payload));
+      return {
+          std::make_unique<OutgoingStreamInternalPayload>(std::move(payload))};
 
     default:
       DCHECK(false);  // This should never happen.
-      return {};
+      return {Error(OperationResultCode::DETAIL_UNKNOWN)};
   }
 }
 
@@ -350,26 +353,27 @@ std::string make_path(const std::string& custom_save_path,
   return api::ImplementationPlatform::GetDownloadPath(parent_folder, file_name);
 }
 
-std::unique_ptr<InternalPayload> CreateIncomingInternalPayload(
+ErrorOr<std::unique_ptr<InternalPayload>> CreateIncomingInternalPayload(
     const location::nearby::connections::PayloadTransferFrame& frame,
     const std::string& custom_save_path) {
   if (frame.packet_type() !=
       location::nearby::connections::PayloadTransferFrame::DATA) {
-    return {};
+    return {Error(
+        OperationResultCode::NEARBY_GENERIC_INCOMING_PAYLOAD_NOT_DATA_TYPE)};
   }
 
   const Payload::Id payload_id = frame.payload_header().id();
   switch (frame.payload_header().type()) {
     case PayloadTransferFrame::PayloadHeader::BYTES: {
-      return std::make_unique<BytesInternalPayload>(
-          Payload(payload_id, ByteArray(frame.payload_chunk().body())));
+      return {std::make_unique<BytesInternalPayload>(
+          Payload(payload_id, ByteArray(frame.payload_chunk().body())))};
     }
 
     case PayloadTransferFrame::PayloadHeader::STREAM: {
       auto [input, output] = CreatePipe();
 
-      return std::make_unique<IncomingStreamInternalPayload>(
-          Payload(payload_id, std::move(input)), std::move(output));
+      return {std::make_unique<IncomingStreamInternalPayload>(
+          Payload(payload_id, std::move(input)), std::move(output))};
     }
 
     case PayloadTransferFrame::PayloadHeader::FILE: {
@@ -397,7 +401,7 @@ std::unique_ptr<InternalPayload> CreateIncomingInternalPayload(
           // file name for the output file.
           NEARBY_LOGS(ERROR) << "File name not found in incoming file Payload, "
                                 "and the Id wasn't found.";
-          return {};
+          return {Error(OperationResultCode::IO_FILE_OPENING_ERROR)};
         }
       }
 
@@ -409,19 +413,19 @@ std::unique_ptr<InternalPayload> CreateIncomingInternalPayload(
       // there will be no input file to open.
       // On Chrome the file path should be empty, so use the payload id.
       if (ImplementationPlatform::GetCurrentOS() == OSName::kChromeOS) {
-        return std::make_unique<IncomingFileInternalPayload>(
+        return {std::make_unique<IncomingFileInternalPayload>(
             Payload(payload_id, InputFile(payload_id, total_size)),
-            OutputFile(payload_id), total_size);
+            OutputFile(payload_id), total_size)};
       } else {
-        return std::make_unique<IncomingFileInternalPayload>(
+        return {std::make_unique<IncomingFileInternalPayload>(
             Payload(payload_id, parent_folder, file_name,
                     InputFile(file_path, total_size)),
-            OutputFile(file_path), total_size);
+            OutputFile(file_path), total_size)};
       }
     }
     default:
       DCHECK(false);  // This should never happen.
-      return {};
+      return {Error(OperationResultCode::DETAIL_UNKNOWN)};
   }
 }
 

connections/implementation/internal_payload_factory.h

@@ -15,22 +15,23 @@
 #ifndef CORE_INTERNAL_INTERNAL_PAYLOAD_FACTORY_H_
 #define CORE_INTERNAL_INTERNAL_PAYLOAD_FACTORY_H_
 
-#include <string>
-
 #include <memory>
+#include <string>
 
 #include "connections/implementation/internal_payload.h"
 #include "connections/payload.h"
+#include "internal/platform/expected.h"
 
 namespace nearby {
 namespace connections {
 
 // Creates an InternalPayload representing an outgoing Payload.
-std::unique_ptr<InternalPayload> CreateOutgoingInternalPayload(Payload payload);
+ErrorOr<std::unique_ptr<InternalPayload>> CreateOutgoingInternalPayload(
+    Payload payload);
 
 // Creates an InternalPayload representing an incoming Payload from a remote
 // endpoint.
-std::unique_ptr<InternalPayload> CreateIncomingInternalPayload(
+ErrorOr<std::unique_ptr<InternalPayload>> CreateIncomingInternalPayload(
     const location::nearby::connections::PayloadTransferFrame& frame,
     const std::string& custom_save_path);
 

connections/implementation/internal_payload_factory_test.cc

@@ -14,6 +14,7 @@
 
 #include "connections/implementation/internal_payload_factory.h"
 
+#include <cstddef>
 #include <cstdint>
 #include <memory>
 #include <string>
@@ -26,6 +27,7 @@
 #include "connections/payload_type.h"
 #include "internal/platform/byte_array.h"
 #include "internal/platform/exception.h"
+#include "internal/platform/expected.h"
 #include "internal/platform/file.h"
 #include "internal/platform/pipe.h"
 
@@ -38,8 +40,10 @@ constexpr char kText[] = "data chunk";
 
 TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromBytePayload) {
   ByteArray data(kText);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateOutgoingInternalPayload(Payload{data});
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_EQ(payload.AsFile(), nullptr);
@@ -49,8 +53,10 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromBytePayload) {
 
 TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromStreamPayload) {
   auto [input, output] = CreatePipe();
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateOutgoingInternalPayload(Payload(std::move(input)));
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_EQ(payload.AsFile(), nullptr);
@@ -61,8 +67,10 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromStreamPayload) {
 TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromFilePayload) {
   Payload::Id payload_id = Payload::GenerateId();
   InputFile inputFile(payload_id, 512);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)});
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_NE(payload.AsFile(), nullptr);
@@ -86,8 +94,10 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromByteMessage) {
   header.set_id(12345);
   header.set_total_size(512);
   *frame.mutable_payload_chunk() = std::move(payload_chunk);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_EQ(payload.AsFile(), nullptr);
@@ -103,8 +113,10 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromStreamMessage) {
   header.set_type(PayloadTransferFrame::PayloadHeader::STREAM);
   header.set_id(12345);
   header.set_total_size(0);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   {
     Payload payload = internal_payload->ReleasePayload();
@@ -126,8 +138,10 @@ TEST(InternalPayloadFactoryTest, CanCreateInternalPayloadFromFileMessage) {
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
   header.set_id(12345);
   header.set_total_size(512);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_NE(payload.AsFile(), nullptr);
@@ -144,9 +158,9 @@ TEST(InternalPayloadFactoryTest,
   auto& header = *frame.mutable_payload_header();
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
   header.set_total_size(512);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, path);
-  EXPECT_EQ(internal_payload, nullptr);
+  EXPECT_TRUE(result.has_error());
 }
 
 TEST(InternalPayloadFactoryTest,
@@ -158,8 +172,10 @@ TEST(InternalPayloadFactoryTest,
   header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
   header.set_id(12345);
   header.set_total_size(512);
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_EQ(payload.GetFileName(), "12345");
@@ -174,8 +190,10 @@ TEST(InternalPayloadFactoryTest,
   header.set_id(12345);
   header.set_total_size(512);
   header.set_file_name("test.file.name");
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, path);
+  ASSERT_FALSE(result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   EXPECT_NE(internal_payload, nullptr);
   auto test = internal_payload->GetFileName();
   Payload payload = internal_payload->ReleasePayload();
@@ -196,8 +214,11 @@ TEST(InternalPayloadFactoryTest,
   Payload::Id payload_id = Payload::GenerateId();
   CreateFileWithContents(payload_id, contents);
   InputFile inputFile(payload_id, contents.size());
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> interal_payload_result =
       CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)});
+  ASSERT_FALSE(interal_payload_result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload =
+      std::move(interal_payload_result.value());
   EXPECT_NE(internal_payload, nullptr);
 
   ExceptionOr<size_t> result = internal_payload->SkipToOffset(kOffset);
@@ -215,8 +236,11 @@ TEST(InternalPayloadFactoryTest,
   ByteArray contents("0123456789");
   constexpr size_t kOffset = 6;
   auto [input, output] = CreatePipe();
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> interal_payload_result =
       CreateOutgoingInternalPayload(Payload(std::move(input)));
+  ASSERT_FALSE(interal_payload_result.has_error());
+  std::unique_ptr<InternalPayload> internal_payload =
+      std::move(interal_payload_result.value());
   EXPECT_NE(internal_payload, nullptr);
   output->Write(contents);
 

connections/implementation/payload_manager.cc

@@ -305,7 +305,14 @@ std::string PayloadManager::ToString(EndpointInfo::Status status) {
 // Creates and starts tracking a PendingPayload for this Payload.
 Payload::Id PayloadManager::CreateOutgoingPayload(
     Payload payload, const EndpointIds& endpoint_ids) {
-  auto internal_payload{CreateOutgoingInternalPayload(std::move(payload))};
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
+      CreateOutgoingInternalPayload(std::move(payload));
+  if (result.has_error()) {
+    LOG(ERROR) << "Failed to create outgoing internal payload: "
+               << result.error().operation_result_code().value();
+    return Payload::Id();
+  }
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   Payload::Id payload_id = internal_payload->GetId();
   LOG(INFO) << "CreateOutgoingPayload: payload_id=" << payload_id;
   MutexLock lock(&mutex_);
@@ -789,13 +796,12 @@ PayloadTransferFrame::PayloadChunk PayloadManager::CreatePayloadChunk(
 ErrorOr<PayloadManager::PendingPayloadHandle>
 PayloadManager::CreateIncomingPayload(const PayloadTransferFrame& frame,
                                       const std::string& endpoint_id) {
-  // TODO(edwinwu): Add for return result code in CreateIncomingInternalPayload.
-  std::unique_ptr<InternalPayload> internal_payload =
+  ErrorOr<std::unique_ptr<InternalPayload>> result =
       CreateIncomingInternalPayload(frame, custom_save_path_);
-  if (!internal_payload) {
-    return {Error(OperationResultCode::DETAIL_UNKNOWN)};
+  if (result.has_error()) {
+    return {result.error()};
   }
-
+  std::unique_ptr<InternalPayload> internal_payload = std::move(result.value());
   Payload::Id payload_id = internal_payload->GetId();
   LOG(INFO) << "CreateIncomingPayload: payload_id=" << payload_id;
   pending_payloads_.StartTrackingPayload(