diff --git a/connections/implementation/analytics/BUILD b/connections/implementation/analytics/BUILD index 2f9435e956..f3a4023ea2 100644 --- a/connections/implementation/analytics/BUILD +++ b/connections/implementation/analytics/BUILD @@ -57,6 +57,7 @@ cc_test( shard_count = 16, deps = [ ":analytics", + "//connections:core_types", "//internal/analytics:mock_event_logger", "//internal/platform:base", "//internal/platform:error_code_recorder", diff --git a/connections/implementation/analytics/analytics_recorder.cc b/connections/implementation/analytics/analytics_recorder.cc index daa7bc213e..d3ab8bb824 100644 --- a/connections/implementation/analytics/analytics_recorder.cc +++ b/connections/implementation/analytics/analytics_recorder.cc @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -39,7 +38,6 @@ #include "internal/platform/single_thread_executor.h" #include "internal/proto/analytics/connections_log.pb.h" #include "proto/connections_enums.pb.h" -#include "google/protobuf/repeated_ptr_field.h" namespace nearby { namespace analytics { @@ -48,7 +46,6 @@ namespace { const char kVersion[] = "v1.0.0"; constexpr absl::string_view kOnStartClientSession = "OnStartClientSession"; const absl::Duration kConnectionTokenMaxLife = absl::Hours(24); -} // namespace using ::location::nearby::analytics::proto::ConnectionsLog; using ::location::nearby::proto::connections::ACCEPTED; @@ -76,6 +73,8 @@ using ::location::nearby::proto::connections::INITIAL; using ::location::nearby::proto::connections::Medium; using ::location::nearby::proto::connections::MOVED_TO_NEW_MEDIUM; using ::location::nearby::proto::connections::NOT_SENT; +using ::location::nearby::proto::connections::OperationResultCategory; +using ::location::nearby::proto::connections::OperationResultCode; using ::location::nearby::proto::connections::OUTGOING; using ::location::nearby::proto::connections::P2P_CLUSTER; using ::location::nearby::proto::connections::P2P_POINT_TO_POINT; @@ -103,6 +102,57 @@ using ::nearby::analytics::EventLogger; using SafeDisconnectionResult = ::location::nearby::analytics::proto:: ConnectionsLog::EstablishedConnection::SafeDisconnectionResult; +// TODO(edwinwu): Add ifttt in Android counterpart. +OperationResultCategory GetOperationResultCateory( + OperationResultCode result_code) { + if (result_code == OperationResultCode::DETAIL_SUCCESS) { + return OperationResultCategory::CATEGORY_SUCCESS; + } + // Section of CATEGORY_NEARBY_ERROR, starting from 4500 + if (result_code >= + OperationResultCode::NEARBY_BLE_ADVERTISEMENT_MAPPING_TO_MAC_ERROR) { + return OperationResultCategory::CATEGORY_NEARBY_ERROR; + } + // Section of CATEGORY_CONNECTIVITY_ERROR, starting from 3500 to 4499 + if (result_code >= + OperationResultCode::CONNECTIVITY_WIFI_AWARE_ATTACH_FAILURE) { + return OperationResultCategory::CATEGORY_CONNECTIVITY_ERROR; + } + // Section of CATEGORY_IO_ERROR, from 3000 to 3499 + if (result_code >= OperationResultCode::IO_FILE_OPENING_ERROR) { + return OperationResultCategory::CATEGORY_IO_ERROR; + } + // Section of CATEGORY_MISCELLANEOUS, from 2500 to 2999 + if (result_code >= + OperationResultCode::MISCELLEANEOUS_BLUETOOTH_MAC_ADDRESS_NULL) { + return OperationResultCategory::CATEGORY_MISCELLANEOUS; + } + // Section of CATEGORY_CLIENT_ERROR, from 2000 to 2499 + if (result_code >= + OperationResultCode:: + CLIENT_WIFI_DIRECT_ALREADY_HOSTING_DIRECT_GROUP_FOR_THIS_CLIENT) { + return OperationResultCategory::CATEGORY_CLIENT_ERROR; + } + // Section of CATEGORY_MEDIUM_UNAVAILABLE, from 1500 to 1999 + if (result_code >= OperationResultCode:: + MEDIUM_UNAVAILABLE_WIFI_AWARE_RESOURCE_NOT_AVAILABLE) { + return OperationResultCategory::CATEGORY_MEDIUM_UNAVAILABLE; + } + // Section of CATEGORY_DEVICE_STATE_ERROR, from 1000 to 1499 + if (result_code >= + OperationResultCode::DEVICE_STATE_ERROR_UNFINISHED_UPGRADE_ATTEMPTS) { + return OperationResultCategory::CATEGORY_DEVICE_STATE_ERROR; + } + // Section of CATEGORY_CLIENT_CANCELLATION, from 500 to 999 + if (result_code >= + OperationResultCode::CLIENT_CANCELLATION_REMOTE_IN_CANCELED_STATE) { + return OperationResultCategory::CATEGORY_CLIENT_CANCELLATION; + } + // Clarify other non success cases as unknown + return OperationResultCategory::CATEGORY_UNKNOWN; +} +} // namespace + AnalyticsRecorder::AnalyticsRecorder(EventLogger *event_logger) : event_logger_(event_logger) { NEARBY_LOGS(INFO) << "Start AnalyticsRecorder ctor event_logger_=" @@ -110,6 +160,15 @@ AnalyticsRecorder::AnalyticsRecorder(EventLogger *event_logger) LogStartSession(); } +AnalyticsRecorder::AnalyticsRecorder(EventLogger *event_logger, + bool no_record_time_millis) + : event_logger_(event_logger), + no_record_time_millis_(no_record_time_millis) { + NEARBY_LOGS(INFO) << "Start AnalyticsRecorder ctor event_logger_=" + << event_logger_; + LogStartSession(); +} + AnalyticsRecorder::~AnalyticsRecorder() { serial_executor_.Shutdown(); MutexLock lock(&mutex_); @@ -253,8 +312,10 @@ void AnalyticsRecorder::OnEndpointFound(Medium medium) { ConnectionsLog::DiscoveredEndpoint *discovered_endpoint = current_discovery_phase_->add_discovered_endpoint(); discovered_endpoint->set_medium(medium); - discovered_endpoint->set_latency_millis(absl::ToInt64Milliseconds( - SystemClock::ElapsedRealtime() - started_discovery_phase_time_)); + if (!no_record_time_millis_) { + discovered_endpoint->set_latency_millis(absl::ToInt64Milliseconds( + SystemClock::ElapsedRealtime() - started_discovery_phase_time_)); + } } void AnalyticsRecorder::OnRequestConnection( @@ -279,9 +340,11 @@ void AnalyticsRecorder::OnConnectionRequestReceived( absl::Time current_time = SystemClock::ElapsedRealtime(); auto connection_request = std::make_unique(); - connection_request->set_duration_millis(absl::ToUnixMillis(current_time)); - connection_request->set_request_delay_millis(absl::ToInt64Milliseconds( - current_time - started_advertising_phase_time_)); + if (!no_record_time_millis_) { + connection_request->set_duration_millis(absl::ToUnixMillis(current_time)); + connection_request->set_request_delay_millis(absl::ToInt64Milliseconds( + current_time - started_advertising_phase_time_)); + } incoming_connection_requests_.insert( {remote_endpoint_id, std::move(connection_request)}); } @@ -295,9 +358,11 @@ void AnalyticsRecorder::OnConnectionRequestSent( absl::Time current_time = SystemClock::ElapsedRealtime(); auto connection_request = std::make_unique(); - connection_request->set_duration_millis(absl::ToUnixMillis(current_time)); - connection_request->set_request_delay_millis( - absl::ToInt64Milliseconds(current_time - started_discovery_phase_time_)); + if (!no_record_time_millis_) { + connection_request->set_duration_millis(absl::ToUnixMillis(current_time)); + connection_request->set_request_delay_millis(absl::ToInt64Milliseconds( + current_time - started_discovery_phase_time_)); + } outgoing_connection_requests_.insert( {remote_endpoint_id, std::move(connection_request)}); } @@ -338,7 +403,7 @@ void AnalyticsRecorder::OnLocalEndpointRejected( LocalEndpointRespondedLocked(remote_endpoint_id, REJECTED); } -void AnalyticsRecorder::OnIncomingConnectionAttempt( +void AnalyticsRecorder::OnIncomingConnectionAttemptWithMetadata( ConnectionAttemptType type, Medium medium, ConnectionAttemptResult result, absl::Duration duration, const std::string &connection_token, ConnectionAttemptMetadataParams *connection_attempt_metadata_params) { @@ -351,19 +416,55 @@ void AnalyticsRecorder::OnIncomingConnectionAttempt( "null current_strategy_session_"; return; } + + ConnectionAttemptMetadataParams default_params = {}; + if (connection_attempt_metadata_params == nullptr) { + connection_attempt_metadata_params = &default_params; + } + OnIncomingConnectionAttemptLocked(type, medium, result, duration, + connection_token, + connection_attempt_metadata_params); +} + +void AnalyticsRecorder::OnIncomingConnectionAttempt( + ConnectionAttemptType type, Medium medium, ConnectionAttemptResult result, + absl::Duration duration, const std::string &connection_token, + OperationResultCode operation_result_code) { + MutexLock lock(&mutex_); + if (!CanRecordAnalyticsLocked("OnIncomingConnectionAttempt")) { + return; + } + if (current_strategy_session_ == nullptr) { + NEARBY_LOGS(INFO) << "Unable to record incoming connection attempt due to " + "null current_strategy_session_"; + return; + } + + ConnectionAttemptMetadataParams default_params = {}; + default_params.operation_result_code = operation_result_code; + + OnIncomingConnectionAttemptLocked(type, medium, result, duration, + connection_token, &default_params); +} + +void AnalyticsRecorder::OnIncomingConnectionAttemptLocked( + location::nearby::proto::connections::ConnectionAttemptType type, + location::nearby::proto::connections::Medium medium, + location::nearby::proto::connections::ConnectionAttemptResult result, + absl::Duration duration, const std::string &connection_token, + ConnectionAttemptMetadataParams *connection_attempt_metadata_params) { auto *connection_attempt = current_strategy_session_->add_connection_attempt(); - connection_attempt->set_duration_millis(absl::ToInt64Milliseconds(duration)); + if (!no_record_time_millis_) { + connection_attempt->set_duration_millis( + absl::ToInt64Milliseconds(duration)); + } connection_attempt->set_type(type); connection_attempt->set_direction(INCOMING); connection_attempt->set_medium(medium); connection_attempt->set_attempt_result(result); connection_attempt->set_connection_token(connection_token); - ConnectionAttemptMetadataParams default_params = {}; - if (connection_attempt_metadata_params == nullptr) { - connection_attempt_metadata_params = &default_params; - } auto *connection_attempt_metadata = connection_attempt->mutable_connection_attempt_metadata(); connection_attempt_metadata->set_technology( @@ -390,9 +491,18 @@ void AnalyticsRecorder::OnIncomingConnectionAttempt( connection_attempt_metadata_params->max_wifi_rx_speed); connection_attempt_metadata->set_wifi_channel_width( connection_attempt_metadata_params->channel_width); + + auto operation_result_proto = + std::make_unique(); + operation_result_proto->set_result_code( + connection_attempt_metadata_params->operation_result_code); + operation_result_proto->set_result_category(GetOperationResultCateory( + connection_attempt_metadata_params->operation_result_code)); + connection_attempt->set_allocated_operation_result( + operation_result_proto.release()); } -void AnalyticsRecorder::OnOutgoingConnectionAttempt( +void AnalyticsRecorder::OnOutgoingConnectionAttemptWithMetadata( const std::string &remote_endpoint_id, ConnectionAttemptType type, Medium medium, ConnectionAttemptResult result, absl::Duration duration, const std::string &connection_token, @@ -406,19 +516,74 @@ void AnalyticsRecorder::OnOutgoingConnectionAttempt( "null current_strategy_session_"; return; } + + ConnectionAttemptMetadataParams default_params = {}; + if (connection_attempt_metadata_params == nullptr) { + connection_attempt_metadata_params = &default_params; + } + + // For the case of transfer a big file and the upgrades always failure, then + // there will have repeating upgrade attempt and cause many same attempt value + // be log. So add a method to skip. + if (ConnectionAttemptResultCodeExistedLocked( + medium, OUTGOING, connection_token, type, + connection_attempt_metadata_params->operation_result_code)) { + return; + } + + OnOutgoingConnectionAttemptLocked(remote_endpoint_id, type, medium, result, + duration, connection_token, + connection_attempt_metadata_params); +} + +void AnalyticsRecorder::OnOutgoingConnectionAttempt( + const std::string &remote_endpoint_id, ConnectionAttemptType type, + Medium medium, ConnectionAttemptResult result, absl::Duration duration, + const std::string &connection_token, + OperationResultCode operation_result_code) { + MutexLock lock(&mutex_); + if (!CanRecordAnalyticsLocked("OnOutgoingConnectionAttempt")) { + return; + } + if (current_strategy_session_ == nullptr) { + NEARBY_LOGS(INFO) << "Unable to record outgoing connection attempt due to " + "null current_strategy_session_"; + return; + } + + ConnectionAttemptMetadataParams default_params = {}; + default_params.operation_result_code = operation_result_code; + + // For the case of transfer a big file and the upgrades always failure, then + // there will have repeating upgrade attempt and cause many same attempt value + // be log. So add a method to skip. + if (ConnectionAttemptResultCodeExistedLocked( + medium, OUTGOING, connection_token, type, operation_result_code)) { + return; + } + + OnOutgoingConnectionAttemptLocked(remote_endpoint_id, type, medium, result, + duration, connection_token, + &default_params); +} + +void AnalyticsRecorder::OnOutgoingConnectionAttemptLocked( + const std::string &remote_endpoint_id, ConnectionAttemptType type, + Medium medium, ConnectionAttemptResult result, absl::Duration duration, + const std::string &connection_token, + ConnectionAttemptMetadataParams *connection_attempt_metadata_params) { auto *connection_attempt = current_strategy_session_->add_connection_attempt(); - connection_attempt->set_duration_millis(absl::ToInt64Milliseconds(duration)); + if (!no_record_time_millis_) { + connection_attempt->set_duration_millis( + absl::ToInt64Milliseconds(duration)); + } connection_attempt->set_type(type); connection_attempt->set_direction(OUTGOING); connection_attempt->set_medium(medium); connection_attempt->set_attempt_result(result); connection_attempt->set_connection_token(connection_token); - ConnectionAttemptMetadataParams default_params = {}; - if (connection_attempt_metadata_params == nullptr) { - connection_attempt_metadata_params = &default_params; - } auto *connection_attempt_metadata = connection_attempt->mutable_connection_attempt_metadata(); connection_attempt_metadata->set_technology( @@ -446,6 +611,15 @@ void AnalyticsRecorder::OnOutgoingConnectionAttempt( connection_attempt_metadata->set_wifi_channel_width( connection_attempt_metadata_params->channel_width); + auto operation_result_proto = + std::make_unique(); + operation_result_proto->set_result_code( + connection_attempt_metadata_params->operation_result_code); + operation_result_proto->set_result_category(GetOperationResultCateory( + connection_attempt_metadata_params->operation_result_code)); + connection_attempt->set_allocated_operation_result( + operation_result_proto.release()); + if (type == INITIAL && result != RESULT_SUCCESS) { auto it = outgoing_connection_requests_.find(remote_endpoint_id); if (it != outgoing_connection_requests_.end()) { @@ -475,8 +649,8 @@ void AnalyticsRecorder::OnConnectionEstablished( logical_connection->PhysicalConnectionEstablished(medium, connection_token); } else { active_connections_.insert( - {endpoint_id, - std::make_unique(medium, connection_token)}); + {endpoint_id, std::make_unique( + medium, connection_token, no_record_time_millis_)}); } } @@ -553,9 +727,9 @@ void AnalyticsRecorder::OnPayloadChunkReceived(const std::string &endpoint_id, logical_connection->ChunkReceived(payload_id, chunk_size_bytes); } -void AnalyticsRecorder::OnIncomingPayloadDone(const std::string &endpoint_id, - std::int64_t payload_id, - PayloadStatus status) { +void AnalyticsRecorder::OnIncomingPayloadDone( + const std::string &endpoint_id, std::int64_t payload_id, + PayloadStatus status, OperationResultCode operation_result_code) { MutexLock lock(&mutex_); if (!CanRecordAnalyticsLocked("OnIncomingPayloadDone")) { return; @@ -565,7 +739,8 @@ void AnalyticsRecorder::OnIncomingPayloadDone(const std::string &endpoint_id, return; } const std::unique_ptr &logical_connection = it->second; - logical_connection->IncomingPayloadDone(payload_id, status); + logical_connection->IncomingPayloadDone(payload_id, status, + operation_result_code); } void AnalyticsRecorder::OnOutgoingPayloadStarted( @@ -601,9 +776,9 @@ void AnalyticsRecorder::OnPayloadChunkSent(const std::string &endpoint_id, logical_connection->ChunkSent(payload_id, chunk_size_bytes); } -void AnalyticsRecorder::OnOutgoingPayloadDone(const std::string &endpoint_id, - std::int64_t payload_id, - PayloadStatus status) { +void AnalyticsRecorder::OnOutgoingPayloadDone( + const std::string &endpoint_id, std::int64_t payload_id, + PayloadStatus status, OperationResultCode operation_result_code) { MutexLock lock(&mutex_); if (!CanRecordAnalyticsLocked("OnOutgoingPayloadDone")) { return; @@ -612,8 +787,10 @@ void AnalyticsRecorder::OnOutgoingPayloadDone(const std::string &endpoint_id, if (it == active_connections_.end()) { return; } + const std::unique_ptr &logical_connection = it->second; - logical_connection->OutgoingPayloadDone(payload_id, status); + logical_connection->OutgoingPayloadDone(payload_id, status, + operation_result_code); } void AnalyticsRecorder::OnBandwidthUpgradeStarted( @@ -625,8 +802,10 @@ void AnalyticsRecorder::OnBandwidthUpgradeStarted( } auto bandwidth_upgrade_attempt = std::make_unique(); - bandwidth_upgrade_attempt->set_duration_millis( - absl::ToUnixMillis(SystemClock::ElapsedRealtime())); + if (!no_record_time_millis_) { + bandwidth_upgrade_attempt->set_duration_millis( + absl::ToUnixMillis(SystemClock::ElapsedRealtime())); + } bandwidth_upgrade_attempt->set_from_medium(from_medium); bandwidth_upgrade_attempt->set_to_medium(to_medium); bandwidth_upgrade_attempt->set_direction(direction); @@ -637,12 +816,19 @@ void AnalyticsRecorder::OnBandwidthUpgradeStarted( void AnalyticsRecorder::OnBandwidthUpgradeError( const std::string &endpoint_id, BandwidthUpgradeResult result, - BandwidthUpgradeErrorStage error_stage) { + BandwidthUpgradeErrorStage error_stage, + OperationResultCode operation_result_code) { MutexLock lock(&mutex_); if (!CanRecordAnalyticsLocked("OnBandwidthUpgradeError")) { return; } - FinishUpgradeAttemptLocked(endpoint_id, result, error_stage); + // If the same records existed, drop this one. + if (EraseIfBandwidthUpgradeRecordExistedLocked( + endpoint_id, result, error_stage, operation_result_code)) { + return; + } + FinishUpgradeAttemptLocked(endpoint_id, result, error_stage, + operation_result_code); } void AnalyticsRecorder::OnBandwidthUpgradeSuccess( @@ -652,7 +838,8 @@ void AnalyticsRecorder::OnBandwidthUpgradeSuccess( return; } FinishUpgradeAttemptLocked(endpoint_id, UPGRADE_RESULT_SUCCESS, - UPGRADE_SUCCESS); + UPGRADE_SUCCESS, + OperationResultCode::DETAIL_SUCCESS); } void AnalyticsRecorder::OnErrorCode(const ErrorCodeParams ¶ms) { @@ -744,8 +931,10 @@ void AnalyticsRecorder::LogSession() { return; } FinishStrategySessionLocked(); - client_session_->set_duration_millis(absl::ToInt64Milliseconds( - SystemClock::ElapsedRealtime() - started_client_session_time_)); + if (!no_record_time_millis_) { + client_session_->set_duration_millis(absl::ToInt64Milliseconds( + SystemClock::ElapsedRealtime() - started_client_session_time_)); + } LogClientSessionLocked(); LogEvent(STOP_CLIENT_SESSION); session_was_logged_ = true; @@ -757,7 +946,7 @@ AnalyticsRecorder::BuildConnectionAttemptMetadataParams( int try_count, const std::string &network_operator, const std::string &country_code, bool is_tdls_used, bool wifi_hotspot_enabled, int max_wifi_tx_speed, int max_wifi_rx_speed, - int channel_width) { + int channel_width, OperationResultCode operation_result_code) { auto params = std::make_unique(); params->technology = technology; params->band = band; @@ -770,9 +959,37 @@ AnalyticsRecorder::BuildConnectionAttemptMetadataParams( params->max_wifi_tx_speed = max_wifi_tx_speed; params->max_wifi_rx_speed = max_wifi_rx_speed; params->channel_width = channel_width; + params->operation_result_code = operation_result_code; return params; } +OperationResultCode AnalyticsRecorder::GetChannelIoErrorResultCodeFromMedium( + Medium medium) { + switch (medium) { + case Medium::BLUETOOTH: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_BT; + case Medium::WIFI_HOTSPOT: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_WIFI_HOTSPOT; + case Medium::BLE: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_BLE; + case Medium::BLE_L2CAP: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_BLE_L2CAP; + case Medium::WIFI_LAN: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_LAN; + case Medium::WIFI_AWARE: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_WIFI_AWARE; + case Medium::NFC: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_NFC; + case Medium::WIFI_DIRECT: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_WIFI_DIRECT; + case Medium::WEB_RTC: + return OperationResultCode::CONNECTIVITY_CHANNEL_IO_ERROR_ON_WEB_RTC; + default: + return OperationResultCode:: + CONNECTIVITY_CHANNEL_IO_ERROR_ON_UNKNOWN_MEDIUM; + } +} + bool AnalyticsRecorder::CanRecordAnalyticsLocked( absl::string_view method_name) { NEARBY_VLOG(1) << "AnalyticsRecorder LogEvent " << method_name @@ -861,7 +1078,8 @@ void AnalyticsRecorder::RecordAdvertisingPhaseDurationLocked() const { "null current_advertising_phase_"; return; } - if (!current_advertising_phase_->has_duration_millis()) { + if (!current_advertising_phase_->has_duration_millis() && + !no_record_time_millis_) { current_advertising_phase_->set_duration_millis(absl::ToInt64Milliseconds( SystemClock::ElapsedRealtime() - started_advertising_phase_time_)); } @@ -890,7 +1108,8 @@ void AnalyticsRecorder::RecordDiscoveryPhaseDurationLocked() const { "null current_discovery_phase_"; return; } - if (!current_discovery_phase_->has_duration_millis()) { + if (!current_discovery_phase_->has_duration_millis() && + !no_record_time_millis_) { current_discovery_phase_->set_duration_millis(absl::ToInt64Milliseconds( SystemClock::ElapsedRealtime() - started_discovery_phase_time_)); } @@ -922,9 +1141,11 @@ bool AnalyticsRecorder::UpdateAdvertiserConnectionRequestLocked( return false; } if (BothEndpointsRespondedLocked(request)) { - request->set_duration_millis( - absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - - request->duration_millis()); + if (!no_record_time_millis_) { + request->set_duration_millis( + absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - + request->duration_millis()); + } *current_advertising_phase_->add_received_connection_request() = *request; return true; } @@ -940,9 +1161,11 @@ bool AnalyticsRecorder::UpdateDiscovererConnectionRequestLocked( } if (BothEndpointsRespondedLocked(request) || request->local_response() == NOT_SENT) { - request->set_duration_millis( - absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - - request->duration_millis()); + if (!no_record_time_millis_) { + request->set_duration_millis( + absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - + request->duration_millis()); + } *current_discovery_phase_->add_sent_connection_request() = *request; return true; } @@ -1004,9 +1227,62 @@ void AnalyticsRecorder::MarkConnectionRequestIgnoredLocked( } } +bool AnalyticsRecorder::ConnectionAttemptResultCodeExistedLocked( + Medium medium, ConnectionAttemptDirection direction, + const std::string &connection_token, ConnectionAttemptType type, + OperationResultCode operation_result_code) { + if (current_strategy_session_ == nullptr || + current_strategy_session_->connection_attempt_size() == 0) { + return false; + } + for (auto &connection_attempt : + current_strategy_session_->connection_attempt()) { + if (connection_attempt.medium() == medium && + connection_attempt.direction() == direction && + connection_attempt.connection_token() == connection_token && + connection_attempt.type() == type && + connection_attempt.operation_result().result_code() == + operation_result_code) { + return true; + } + } + + return false; +} + +// If bandwidth upgrade always failed on the same fromMedium, toMedium, result, +// stage and result code, we'll drop the duplicate logs for preventing the waste +// of log storage space +bool AnalyticsRecorder::EraseIfBandwidthUpgradeRecordExistedLocked( + const std::string &endpoint_id, BandwidthUpgradeResult result, + BandwidthUpgradeErrorStage error_stage, + OperationResultCode operation_result_code) { + if (current_strategy_session_ == nullptr) { + return false; + } + auto it = bandwidth_upgrade_attempts_.find(endpoint_id); + if (it != bandwidth_upgrade_attempts_.end()) { + ConnectionsLog::BandwidthUpgradeAttempt *attempt = it->second.get(); + for (auto &existing_attempt : + current_strategy_session_->upgrade_attempt()) { + if (attempt->from_medium() == existing_attempt.from_medium() && + attempt->to_medium() == existing_attempt.to_medium() && + result == existing_attempt.upgrade_result() && + error_stage == existing_attempt.error_stage() && + operation_result_code == + existing_attempt.operation_result().result_code()) { + bandwidth_upgrade_attempts_.erase(it); + return true; + } + } + } + return false; +} + void AnalyticsRecorder::FinishUpgradeAttemptLocked( const std::string &endpoint_id, BandwidthUpgradeResult result, - BandwidthUpgradeErrorStage error_stage, bool erase_item) { + BandwidthUpgradeErrorStage error_stage, + OperationResultCode operation_result_code, bool erase_item) { if (current_strategy_session_ == nullptr) { NEARBY_LOGS(INFO) << "Unable to record upgrade attempt due to null " "current_strategy_session_"; @@ -1016,11 +1292,20 @@ void AnalyticsRecorder::FinishUpgradeAttemptLocked( auto it = bandwidth_upgrade_attempts_.find(endpoint_id); if (it != bandwidth_upgrade_attempts_.end()) { ConnectionsLog::BandwidthUpgradeAttempt *attempt = it->second.get(); - attempt->set_duration_millis( - absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - - attempt->duration_millis()); + if (!no_record_time_millis_) { + attempt->set_duration_millis( + absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - + attempt->duration_millis()); + } attempt->set_error_stage(error_stage); attempt->set_upgrade_result(result); + + auto operation_result_proto = + std::make_unique(); + operation_result_proto->set_result_code(operation_result_code); + operation_result_proto->set_result_category( + GetOperationResultCateory(operation_result_code)); + attempt->set_allocated_operation_result(operation_result_proto.release()); *current_strategy_session_->add_upgrade_attempt() = *attempt; if (erase_item) { bandwidth_upgrade_attempts_.erase(it); @@ -1047,15 +1332,20 @@ void AnalyticsRecorder::FinishStrategySessionLocked() { // Finish any pending upgrade attempts. for (const auto &item : bandwidth_upgrade_attempts_) { - FinishUpgradeAttemptLocked(item.first, UNFINISHED_ERROR, - UPGRADE_UNFINISHED, /*erase_item=*/false); + FinishUpgradeAttemptLocked( + item.first, UNFINISHED_ERROR, UPGRADE_UNFINISHED, + OperationResultCode::DEVICE_STATE_ERROR_UNFINISHED_UPGRADE_ATTEMPTS, + /*erase_item=*/false); } bandwidth_upgrade_attempts_.clear(); // Add the StrategySession in ClientSession if (current_strategy_session_ != nullptr) { - current_strategy_session_->set_duration_millis(absl::ToInt64Milliseconds( - SystemClock::ElapsedRealtime() - started_strategy_session_time_)); + if (!no_record_time_millis_) { + current_strategy_session_->set_duration_millis( + absl::ToInt64Milliseconds(SystemClock::ElapsedRealtime() - + started_strategy_session_time_)); + } *client_session_->add_strategy_session() = *std::move(current_strategy_session_); } @@ -1103,14 +1393,23 @@ void AnalyticsRecorder::PendingPayload::AddChunk( ConnectionsLog::Payload AnalyticsRecorder::PendingPayload::GetProtoPayload( PayloadStatus status) { ConnectionsLog::Payload payload; - payload.set_duration_millis( - absl::ToInt64Milliseconds(SystemClock::ElapsedRealtime() - start_time_)); + if (!no_record_time_millis_) { + payload.set_duration_millis(absl::ToInt64Milliseconds( + SystemClock::ElapsedRealtime() - start_time_)); + } payload.set_type(type_); payload.set_total_size_bytes(total_size_bytes_); payload.set_num_bytes_transferred(num_bytes_transferred_); payload.set_num_chunks(num_chunks_); payload.set_status(status); + auto operation_result_proto = + std::make_unique(); + operation_result_proto->set_result_code(operation_result_code_); + operation_result_proto->set_result_category( + GetOperationResultCateory(operation_result_code_)); + payload.set_allocated_operation_result(operation_result_proto.release()); + return payload; } @@ -1125,9 +1424,19 @@ void AnalyticsRecorder::LogicalConnection::PhysicalConnectionEstablished( auto established_connection = std::make_unique(); established_connection->set_medium(medium); - established_connection->set_duration_millis( - absl::ToUnixMillis(SystemClock::ElapsedRealtime())); + if (!no_record_time_millis_) { + established_connection->set_duration_millis( + absl::ToUnixMillis(SystemClock::ElapsedRealtime())); + } established_connection->set_connection_token(connection_token); + + auto operation_result_proto = + std::make_unique(); + operation_result_proto->set_result_code(OperationResultCode::DETAIL_SUCCESS); + operation_result_proto->set_result_category( + OperationResultCategory::CATEGORY_SUCCESS); + established_connection->set_allocated_operation_result( + operation_result_proto.release()); physical_connections_.insert({medium, std::move(established_connection)}); current_medium_ = medium; } @@ -1215,7 +1524,8 @@ AnalyticsRecorder::LogicalConnection::GetEstablisedConnections() { void AnalyticsRecorder::LogicalConnection::IncomingPayloadStarted( std::int64_t payload_id, PayloadType type, std::int64_t total_size_bytes) { incoming_payloads_.insert( - {payload_id, std::make_unique(type, total_size_bytes)}); + {payload_id, std::make_unique(type, total_size_bytes, + no_record_time_millis_)}); } void AnalyticsRecorder::LogicalConnection::ChunkReceived( @@ -1229,7 +1539,8 @@ void AnalyticsRecorder::LogicalConnection::ChunkReceived( } void AnalyticsRecorder::LogicalConnection::IncomingPayloadDone( - std::int64_t payload_id, PayloadStatus status) { + std::int64_t payload_id, PayloadStatus status, + OperationResultCode operation_result_code) { if (current_medium_ == UNKNOWN_MEDIUM) { NEARBY_LOGS(WARNING) << "Unexpected call to incomingPayloadDone() while " "AnalyticsRecorder has no active current medium."; @@ -1241,6 +1552,7 @@ void AnalyticsRecorder::LogicalConnection::IncomingPayloadDone( &established_connection = it->second; auto it = incoming_payloads_.find(payload_id); if (it != incoming_payloads_.end()) { + it->second->SetOperationResultCode(operation_result_code); *established_connection->add_received_payload() = it->second->GetProtoPayload(status); incoming_payloads_.erase(it); @@ -1251,7 +1563,8 @@ void AnalyticsRecorder::LogicalConnection::IncomingPayloadDone( void AnalyticsRecorder::LogicalConnection::OutgoingPayloadStarted( std::int64_t payload_id, PayloadType type, std::int64_t total_size_bytes) { outgoing_payloads_.insert( - {payload_id, std::make_unique(type, total_size_bytes)}); + {payload_id, std::make_unique(type, total_size_bytes, + no_record_time_millis_)}); } void AnalyticsRecorder::LogicalConnection::ChunkSent(std::int64_t payload_id, @@ -1265,7 +1578,8 @@ void AnalyticsRecorder::LogicalConnection::ChunkSent(std::int64_t payload_id, } void AnalyticsRecorder::LogicalConnection::OutgoingPayloadDone( - std::int64_t payload_id, PayloadStatus status) { + std::int64_t payload_id, PayloadStatus status, + OperationResultCode operation_result_code) { if (current_medium_ == UNKNOWN_MEDIUM) { NEARBY_LOGS(WARNING) << "Unexpected call to outgoingPayloadDone() while " "AnalyticsRecorder has no active current medium."; @@ -1277,6 +1591,7 @@ void AnalyticsRecorder::LogicalConnection::OutgoingPayloadDone( &established_connection = it->second; auto it = outgoing_payloads_.find(payload_id); if (it != outgoing_payloads_.end()) { + it->second->SetOperationResultCode(operation_result_code); *established_connection->add_sent_payload() = it->second->GetProtoPayload(status); outgoing_payloads_.erase(it); @@ -1289,9 +1604,11 @@ void AnalyticsRecorder::LogicalConnection::FinishPhysicalConnection( DisconnectionReason reason, SafeDisconnectionResult result) { established_connection->set_disconnection_reason(reason); established_connection->set_safe_disconnection_result(result); - established_connection->set_duration_millis( - absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - - established_connection->duration_millis()); + if (!no_record_time_millis_) { + established_connection->set_duration_millis( + absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - + established_connection->duration_millis()); + } // Add any not-yet-finished payloads to this EstablishedConnection. std::vector in_payloads = @@ -1316,8 +1633,12 @@ AnalyticsRecorder::LogicalConnection::ResolvePendingPayloads( upgraded_payloads; PayloadStatus status = reason == UPGRADED ? MOVED_TO_NEW_MEDIUM : CONNECTION_CLOSED; + + OperationResultCode operation_result_code = + GetPendingPayloadResultCodeFromReason(reason); for (const auto &item : pending_payloads) { const std::unique_ptr &pending_payload = item.second; + pending_payload->SetOperationResultCode(operation_result_code); ConnectionsLog::Payload proto_payload = pending_payload->GetProtoPayload(status); completed_payloads.push_back(proto_payload); @@ -1325,7 +1646,8 @@ AnalyticsRecorder::LogicalConnection::ResolvePendingPayloads( upgraded_payloads.insert( {item.first, std::make_unique( - pending_payload->type(), pending_payload->total_size_bytes())}); + pending_payload->type(), pending_payload->total_size_bytes(), + no_record_time_millis_, operation_result_code)}); } } pending_payloads.clear(); @@ -1340,6 +1662,21 @@ AnalyticsRecorder::LogicalConnection::ResolvePendingPayloads( return completed_payloads; } +OperationResultCode +AnalyticsRecorder::LogicalConnection::GetPendingPayloadResultCodeFromReason( + DisconnectionReason reason) { + switch (reason) { + case UPGRADED: + return OperationResultCode::MISCELLEANEOUS_MOVE_TO_NEW_MEDIUM; + case DisconnectionReason::LOCAL_DISCONNECTION: + return OperationResultCode::CLIENT_CANCELLATION_LOCAL_DISCONNECT; + case DisconnectionReason::REMOTE_DISCONNECTION: + return OperationResultCode::CLIENT_CANCELLATION_REMOTE_DISCONNECT; + default: + return OperationResultCode::NEARBY_GENERIC_CONNECTION_CLOSED; + } +} + void AnalyticsRecorder::Sync() { CountDownLatch latch(1); serial_executor_.Execute([&]() { latch.CountDown(); }); diff --git a/connections/implementation/analytics/analytics_recorder.h b/connections/implementation/analytics/analytics_recorder.h index 7145f1ac5f..00a3a4bd21 100644 --- a/connections/implementation/analytics/analytics_recorder.h +++ b/connections/implementation/analytics/analytics_recorder.h @@ -42,6 +42,9 @@ namespace analytics { class AnalyticsRecorder { public: explicit AnalyticsRecorder(::nearby::analytics::EventLogger *event_logger); + // For testing only. + AnalyticsRecorder(::nearby::analytics::EventLogger *event_logger, + bool no_record_time_millis); virtual ~AnalyticsRecorder(); // TODO(edwinwu): Implement to pass real values for AdvertisingMetadata and @@ -92,14 +95,26 @@ class AnalyticsRecorder { ABSL_LOCKS_EXCLUDED(mutex_); // Connection attempt - void OnIncomingConnectionAttempt( + // Records an attempt with meta data at establishing an incoming physical + // connection. + void OnIncomingConnectionAttemptWithMetadata( location::nearby::proto::connections::ConnectionAttemptType type, location::nearby::proto::connections::Medium medium, location::nearby::proto::connections::ConnectionAttemptResult result, absl::Duration duration, const std::string &connection_token, ConnectionAttemptMetadataParams *connection_attempt_metadata_params) ABSL_LOCKS_EXCLUDED(mutex_); - void OnOutgoingConnectionAttempt( + // Records an attempt at establishing an incoming physical connection. + void OnIncomingConnectionAttempt( + location::nearby::proto::connections::ConnectionAttemptType type, + location::nearby::proto::connections::Medium medium, + location::nearby::proto::connections::ConnectionAttemptResult result, + absl::Duration duration, const std::string &connection_token, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_LOCKS_EXCLUDED(mutex_); + // Records an attempt with meta data at establishing an outgoing physical + // connection. + void OnOutgoingConnectionAttemptWithMetadata( const std::string &remote_endpoint_id, location::nearby::proto::connections::ConnectionAttemptType type, location::nearby::proto::connections::Medium medium, @@ -107,6 +122,15 @@ class AnalyticsRecorder { absl::Duration duration, const std::string &connection_token, ConnectionAttemptMetadataParams *connection_attempt_metadata_params) ABSL_LOCKS_EXCLUDED(mutex_); + // Records an attempt at establishing an outgoing physical connection. + void OnOutgoingConnectionAttempt( + const std::string &remote_endpoint_id, + location::nearby::proto::connections::ConnectionAttemptType type, + location::nearby::proto::connections::Medium medium, + location::nearby::proto::connections::ConnectionAttemptResult result, + absl::Duration duration, const std::string &connection_token, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_LOCKS_EXCLUDED(mutex_); // TODO(edwinwu): Implement network operator, country code, tdls, wifi hotspot //, max wifi tx/rx speed and channel width. Set as default values for // analytics recorder. @@ -117,7 +141,13 @@ class AnalyticsRecorder { int try_count, const std::string &network_operator = {}, const std::string &country_code = {}, bool is_tdls_used = false, bool wifi_hotspot_enabled = false, int max_wifi_tx_speed = 0, - int max_wifi_rx_speed = 0, int channel_width = -1); + int max_wifi_rx_speed = 0, int channel_width = -1, + location::nearby::proto::connections::OperationResultCode + operation_result_code = location::nearby::proto::connections:: + OperationResultCode::DETAIL_UNKNOWN); + static location::nearby::proto::connections::OperationResultCode + GetChannelIoErrorResultCodeFromMedium( + location::nearby::proto::connections::Medium medium); // Connection establishedSafeDisconnectionResult void OnConnectionEstablished( @@ -144,8 +174,9 @@ class AnalyticsRecorder { ABSL_LOCKS_EXCLUDED(mutex_); void OnIncomingPayloadDone( const std::string &endpoint_id, std::int64_t payload_id, - location::nearby::proto::connections::PayloadStatus status) - ABSL_LOCKS_EXCLUDED(mutex_); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_LOCKS_EXCLUDED(mutex_); void OnOutgoingPayloadStarted(const std::vector &endpoint_ids, std::int64_t payload_id, connections::PayloadType type, @@ -157,8 +188,9 @@ class AnalyticsRecorder { ABSL_LOCKS_EXCLUDED(mutex_); void OnOutgoingPayloadDone( const std::string &endpoint_id, std::int64_t payload_id, - location::nearby::proto::connections::PayloadStatus status) - ABSL_LOCKS_EXCLUDED(mutex_); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_LOCKS_EXCLUDED(mutex_); // BandwidthUpgrade void OnBandwidthUpgradeStarted( @@ -172,7 +204,9 @@ class AnalyticsRecorder { const std::string &endpoint_id, location::nearby::proto::connections::BandwidthUpgradeResult result, location::nearby::proto::connections::BandwidthUpgradeErrorStage - error_stage) ABSL_LOCKS_EXCLUDED(mutex_); + error_stage, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_LOCKS_EXCLUDED(mutex_); void OnBandwidthUpgradeSuccess(const std::string &endpoint_id) ABSL_LOCKS_EXCLUDED(mutex_); @@ -180,7 +214,7 @@ class AnalyticsRecorder { void OnErrorCode(const ErrorCodeParams ¶ms); // Log the start client session event with start client session logging - // resouces setup (e.g. client_session_, started_client_session_time_) + // resources setup (e.g. client_session_, started_client_session_time_) void LogStartSession() ABSL_LOCKS_EXCLUDED(mutex_); // Invokes event_logger_.Log() at the end of life of client. Log action is @@ -199,12 +233,21 @@ class AnalyticsRecorder { class PendingPayload { public: PendingPayload(location::nearby::proto::connections::PayloadType type, - std::int64_t total_size_bytes) + std::int64_t total_size_bytes, bool no_record_time_millis) + : PendingPayload(type, total_size_bytes, no_record_time_millis, + location::nearby::proto::connections:: + OperationResultCode::DETAIL_UNKNOWN) {} + PendingPayload(location::nearby::proto::connections::PayloadType type, + std::int64_t total_size_bytes, bool no_record_time_millis, + location::nearby::proto::connections::OperationResultCode + operation_result_code) : start_time_(SystemClock::ElapsedRealtime()), type_(type), total_size_bytes_(total_size_bytes), num_bytes_transferred_(0), - num_chunks_(0) {} + num_chunks_(0), + operation_result_code_(operation_result_code), + no_record_time_millis_(no_record_time_millis) {} ~PendingPayload() = default; void AddChunk(std::int64_t chunk_size_bytes); @@ -218,19 +261,31 @@ class AnalyticsRecorder { std::int64_t total_size_bytes() const { return total_size_bytes_; } + void SetOperationResultCode( + location::nearby::proto::connections::OperationResultCode + operation_result_code) { + operation_result_code_ = operation_result_code; + } + private: absl::Time start_time_; location::nearby::proto::connections::PayloadType type_; std::int64_t total_size_bytes_; std::int64_t num_bytes_transferred_; int num_chunks_; + location::nearby::proto::connections::OperationResultCode + operation_result_code_ = location::nearby::proto::connections:: + OperationResultCode::DETAIL_UNKNOWN; + // For testing only. + bool no_record_time_millis_ = false; }; class LogicalConnection { public: LogicalConnection( location::nearby::proto::connections::Medium initial_medium, - const std::string &connection_token) { + const std::string &connection_token, bool no_record_time_millis) + : no_record_time_millis_(no_record_time_millis) { PhysicalConnectionEstablished(initial_medium, connection_token); } LogicalConnection(const LogicalConnection &) = delete; @@ -260,7 +315,9 @@ class AnalyticsRecorder { void ChunkReceived(std::int64_t payload_id, std::int64_t size_bytes); void IncomingPayloadDone( std::int64_t payload_id, - location::nearby::proto::connections::PayloadStatus status); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code); void OutgoingPayloadStarted( std::int64_t payload_id, location::nearby::proto::connections::PayloadType type, @@ -268,7 +325,9 @@ class AnalyticsRecorder { void ChunkSent(std::int64_t payload_id, std::int64_t size_bytes); void OutgoingPayloadDone( std::int64_t payload_id, - location::nearby::proto::connections::PayloadStatus status); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code); std::vector @@ -286,6 +345,9 @@ class AnalyticsRecorder { absl::btree_map> &pending_payloads, location::nearby::proto::connections::DisconnectionReason reason); + location::nearby::proto::connections::OperationResultCode + GetPendingPayloadResultCodeFromReason( + location::nearby::proto::connections::DisconnectionReason reason); location::nearby::proto::connections::Medium current_medium_ = location::nearby::proto::connections::UNKNOWN_MEDIUM; @@ -297,6 +359,8 @@ class AnalyticsRecorder { incoming_payloads_; absl::btree_map> outgoing_payloads_; + // For testing only. + bool no_record_time_millis_ = false; }; bool CanRecordAnalyticsLocked(absl::string_view method_name) @@ -338,11 +402,43 @@ class AnalyticsRecorder { void MarkConnectionRequestIgnoredLocked( location::nearby::analytics::proto::ConnectionsLog::ConnectionRequest *request) ABSL_SHARED_LOCKS_REQUIRED(mutex_); + void OnIncomingConnectionAttemptLocked( + location::nearby::proto::connections::ConnectionAttemptType type, + location::nearby::proto::connections::Medium medium, + location::nearby::proto::connections::ConnectionAttemptResult result, + absl::Duration duration, const std::string &connection_token, + ConnectionAttemptMetadataParams *connection_attempt_metadata_params) + ABSL_SHARED_LOCKS_REQUIRED(mutex_); + void OnOutgoingConnectionAttemptLocked( + const std::string &remote_endpoint_id, + location::nearby::proto::connections::ConnectionAttemptType type, + location::nearby::proto::connections::Medium medium, + location::nearby::proto::connections::ConnectionAttemptResult result, + absl::Duration duration, const std::string &connection_token, + ConnectionAttemptMetadataParams *connection_attempt_metadata_params) + ABSL_SHARED_LOCKS_REQUIRED(mutex_); + bool ConnectionAttemptResultCodeExistedLocked( + location::nearby::proto::connections::Medium medium, + location::nearby::proto::connections::ConnectionAttemptDirection + direction, + const std::string &connection_token, + location::nearby::proto::connections::ConnectionAttemptType type, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_SHARED_LOCKS_REQUIRED(mutex_); + bool EraseIfBandwidthUpgradeRecordExistedLocked( + const std::string &endpoint_id, + location::nearby::proto::connections::BandwidthUpgradeResult result, + location::nearby::proto::connections::BandwidthUpgradeErrorStage + error_stage, + location::nearby::proto::connections::OperationResultCode + operation_result_code) ABSL_SHARED_LOCKS_REQUIRED(mutex_); void FinishUpgradeAttemptLocked( const std::string &endpoint_id, location::nearby::proto::connections::BandwidthUpgradeResult result, location::nearby::proto::connections::BandwidthUpgradeErrorStage error_stage, + location::nearby::proto::connections::OperationResultCode + operation_result_code, bool erase_item = true) ABSL_SHARED_LOCKS_REQUIRED(mutex_); void FinishStrategySessionLocked() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); @@ -364,6 +460,9 @@ class AnalyticsRecorder { // Protects all sub-protos reading and writing in ConnectionLog. Mutex mutex_; + // For testing only. + bool no_record_time_millis_ = false; + // ClientSession std::unique_ptr< location::nearby::analytics::proto::ConnectionsLog::ClientSession> diff --git a/connections/implementation/analytics/analytics_recorder_test.cc b/connections/implementation/analytics/analytics_recorder_test.cc index a2bc091edf..ea3e83adb4 100644 --- a/connections/implementation/analytics/analytics_recorder_test.cc +++ b/connections/implementation/analytics/analytics_recorder_test.cc @@ -25,6 +25,8 @@ #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "absl/time/time.h" +#include "connections/payload_type.h" +#include "connections/strategy.h" #include "internal/analytics/mock_event_logger.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/error_code_params.h" @@ -55,6 +57,7 @@ using ::location::nearby::proto::connections::INCOMING; using ::location::nearby::proto::connections::INITIAL; using ::location::nearby::proto::connections::LOCAL_DISCONNECTION; using ::location::nearby::proto::connections::Medium; +using ::location::nearby::proto::connections::OperationResultCode; using ::location::nearby::proto::connections::RESULT_ERROR; using ::location::nearby::proto::connections::RESULT_SUCCESS; using ::location::nearby::proto::connections::START_CLIENT_SESSION; @@ -72,7 +75,6 @@ using ::proto2::contrib::parse_proto::ParseTextProtoOrDie; using ::testing::Contains; using ::protobuf_matchers::EqualsProto; using ::testing::Not; -using ::testing::proto::Partially; constexpr absl::Duration kDefaultTimeout = absl::Milliseconds(1000); @@ -142,7 +144,8 @@ class FakeEventLogger : public MockEventLogger { TEST(AnalyticsRecorderTest, SessionOnlyLoggedOnceWorks) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.LogSession(); analytics_recorder.LogSession(); @@ -159,7 +162,8 @@ TEST(AnalyticsRecorderTest, SetFieldsCorrectlyForNestedAdvertisingCalls) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(strategy, /*mediums=*/{BLE, BLUETOOTH}); analytics_recorder.OnStopAdvertising(); @@ -193,7 +197,7 @@ TEST(AnalyticsRecorderTest, SetFieldsCorrectlyForNestedAdvertisingCalls) { >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + (EqualsProto(strategy_session_proto))); } TEST(AnalyticsRecorderTest, SetFieldsCorrectlyForNestedDiscoveryCalls) { @@ -201,7 +205,8 @@ TEST(AnalyticsRecorderTest, SetFieldsCorrectlyForNestedDiscoveryCalls) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartDiscovery( strategy, /*mediums=*/{BLE, BLUETOOTH}, @@ -245,7 +250,7 @@ TEST(AnalyticsRecorderTest, SetFieldsCorrectlyForNestedDiscoveryCalls) { >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, @@ -255,7 +260,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(strategy, mediums); analytics_recorder.OnStartDiscovery(strategy, mediums); @@ -340,7 +346,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, AdvertiserConnectionRequestsWorks) { @@ -351,7 +357,8 @@ TEST(AnalyticsRecorderTest, AdvertiserConnectionRequestsWorks) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -407,7 +414,7 @@ TEST(AnalyticsRecorderTest, AdvertiserConnectionRequestsWorks) { >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, DiscoveryConnectionRequestsWorks) { @@ -418,7 +425,8 @@ TEST(AnalyticsRecorderTest, DiscoveryConnectionRequestsWorks) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -475,7 +483,7 @@ TEST(AnalyticsRecorderTest, DiscoveryConnectionRequestsWorks) { >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, @@ -486,7 +494,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -533,7 +542,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, @@ -544,7 +553,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -592,19 +602,20 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, SuccessfulIncomingConnectionAttempt) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); analytics_recorder.OnIncomingConnectionAttempt( INITIAL, BLUETOOTH, RESULT_SUCCESS, absl::Duration{}, - /*connection_token=*/"", nullptr); + /*connection_token=*/"", OperationResultCode::DETAIL_SUCCESS); analytics_recorder.OnStopAdvertising(); analytics_recorder.LogSession(); @@ -643,11 +654,15 @@ TEST(AnalyticsRecorderTest, SuccessfulIncomingConnectionAttempt) { max_rx_speed: 0 wifi_channel_width: -1 > + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, @@ -656,7 +671,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); auto connections_attempt_metadata_params = analytics_recorder.BuildConnectionAttemptMetadataParams( @@ -667,11 +683,12 @@ TEST(AnalyticsRecorderTest, /*frequency*/ 2400, /*try_count*/ 0, /*network_operator*/ {}, /*country_code*/ {}, /*is_tdls_used*/ false, /*wifi_hotspot_enabled*/ false, /*max_wifi_tx_speed*/ 0, - /*max_wifi_rx_speed*/ 0, /*channel_width*/ 0); + /*max_wifi_rx_speed*/ 0, /*channel_width*/ 0, + OperationResultCode::CONNECTIVITY_BT_CLIENT_SOCKET_CREATION_FAILURE); analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); analytics_recorder.OnConnectionRequestSent(endpoint_id); - analytics_recorder.OnOutgoingConnectionAttempt( + analytics_recorder.OnOutgoingConnectionAttemptWithMetadata( endpoint_id, INITIAL, BLUETOOTH, RESULT_ERROR, absl::Duration{}, /*connection_token=*/"", connections_attempt_metadata_params.get()); @@ -715,11 +732,15 @@ TEST(AnalyticsRecorderTest, max_rx_speed: 0 wifi_channel_width: 0 > + operation_result < + result_category: CATEGORY_CONNECTIVITY_ERROR + result_code: CONNECTIVITY_BT_CLIENT_SOCKET_CREATION_FAILURE + > > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, UnfinishedEstablishedConnectionsAddedAsUnfinished) { @@ -728,15 +749,16 @@ TEST(AnalyticsRecorderTest, UnfinishedEstablishedConnectionsAddedAsUnfinished) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); analytics_recorder.OnConnectionEstablished(endpoint_id, BLUETOOTH, connection_token); analytics_recorder.OnConnectionClosed( - endpoint_id, BLUETOOTH, UPGRADED, ConnectionsLog::EstablishedConnection:: - UNKNOWN_SAFE_DISCONNECTION_RESULT); + endpoint_id, BLUETOOTH, UPGRADED, + ConnectionsLog::EstablishedConnection::UNKNOWN_SAFE_DISCONNECTION_RESULT); analytics_recorder.OnConnectionEstablished(endpoint_id, WIFI_LAN, connection_token); @@ -761,16 +783,26 @@ TEST(AnalyticsRecorderTest, UnfinishedEstablishedConnectionsAddedAsUnfinished) { medium: BLUETOOTH disconnection_reason: UPGRADED connection_token: "connection_token" + safe_disconnection_result: UNKNOWN_SAFE_DISCONNECTION_RESULT + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > established_connection < medium: WIFI_LAN disconnection_reason: UNFINISHED connection_token: "connection_token" + safe_disconnection_result: SAFE_DISCONNECTION + operation_result { + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + } > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { @@ -780,7 +812,8 @@ TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -798,11 +831,11 @@ TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); - analytics_recorder.OnOutgoingPayloadDone(endpoint_id, payload_id, SUCCESS); - analytics_recorder.OnConnectionClosed(endpoint_id, WIFI_LAN, - LOCAL_DISCONNECTION, - ConnectionsLog::EstablishedConnection:: - SAFE_DISCONNECTION); + analytics_recorder.OnOutgoingPayloadDone(endpoint_id, payload_id, SUCCESS, + OperationResultCode::DETAIL_SUCCESS); + analytics_recorder.OnConnectionClosed( + endpoint_id, WIFI_LAN, LOCAL_DISCONNECTION, + ConnectionsLog::EstablishedConnection::SAFE_DISCONNECTION); analytics_recorder.LogSession(); ASSERT_TRUE(client_session_done_latch.Await(kDefaultTimeout).result()); @@ -829,9 +862,18 @@ TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { num_bytes_transferred: 20 num_chunks: 2 status: MOVED_TO_NEW_MEDIUM + operation_result < + result_category: CATEGORY_MISCELLANEOUS + result_code: MISCELLEANEOUS_MOVE_TO_NEW_MEDIUM + > > disconnection_reason: UPGRADED connection_token: "connection_token" + safe_disconnection_result: SAFE_DISCONNECTION + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > established_connection < medium: WIFI_LAN @@ -841,14 +883,23 @@ TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { num_bytes_transferred: 30 num_chunks: 3 status: SUCCESS + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > disconnection_reason: LOCAL_DISCONNECTION connection_token: "connection_token" + safe_disconnection_result: SAFE_DISCONNECTION + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, UpgradeAttemptWorks) { @@ -859,7 +910,8 @@ TEST(AnalyticsRecorderTest, UpgradeAttemptWorks) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -870,8 +922,9 @@ TEST(AnalyticsRecorderTest, UpgradeAttemptWorks) { analytics_recorder.OnBandwidthUpgradeStarted( endpoint_id_1, BLUETOOTH, WIFI_LAN, INCOMING, connection_token); // Error to upgrade. - analytics_recorder.OnBandwidthUpgradeError(endpoint_id, WIFI_LAN_MEDIUM_ERROR, - WIFI_LAN_SOCKET_CREATION); + analytics_recorder.OnBandwidthUpgradeError( + endpoint_id, WIFI_LAN_MEDIUM_ERROR, WIFI_LAN_SOCKET_CREATION, + OperationResultCode::CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL); // Success to upgrade. analytics_recorder.OnBandwidthUpgradeSuccess(endpoint_id_1); // Upgrade is unfinished. @@ -902,6 +955,10 @@ TEST(AnalyticsRecorderTest, UpgradeAttemptWorks) { upgrade_result: WIFI_LAN_MEDIUM_ERROR error_stage: WIFI_LAN_SOCKET_CREATION connection_token: "connection_token" + operation_result < + result_category: CATEGORY_CONNECTIVITY_ERROR + result_code: CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL + > > upgrade_attempt < direction: INCOMING @@ -910,6 +967,10 @@ TEST(AnalyticsRecorderTest, UpgradeAttemptWorks) { upgrade_result: UPGRADE_RESULT_SUCCESS error_stage: UPGRADE_SUCCESS connection_token: "connection_token" + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > upgrade_attempt { direction: INCOMING @@ -918,11 +979,15 @@ TEST(AnalyticsRecorderTest, UpgradeAttemptWorks) { upgrade_result: UNFINISHED_ERROR error_stage: UPGRADE_UNFINISHED connection_token: "connection_token" + operation_result < + result_category: CATEGORY_DEVICE_STATE_ERROR + result_code: DEVICE_STATE_ERROR_UNFINISHED_UPGRADE_ATTEMPTS + > } >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, StartListeningForIncomingConnectionsWorks) { @@ -933,7 +998,8 @@ TEST(AnalyticsRecorderTest, StartListeningForIncomingConnectionsWorks) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartedIncomingConnectionListening( connections::Strategy::kP2pStar); @@ -944,8 +1010,9 @@ TEST(AnalyticsRecorderTest, StartListeningForIncomingConnectionsWorks) { analytics_recorder.OnBandwidthUpgradeStarted( endpoint_id_1, BLUETOOTH, WIFI_LAN, INCOMING, connection_token); // Error to upgrade. - analytics_recorder.OnBandwidthUpgradeError(endpoint_id, WIFI_LAN_MEDIUM_ERROR, - WIFI_LAN_SOCKET_CREATION); + analytics_recorder.OnBandwidthUpgradeError( + endpoint_id, WIFI_LAN_MEDIUM_ERROR, WIFI_LAN_SOCKET_CREATION, + OperationResultCode::CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL); // Success to upgrade. analytics_recorder.OnBandwidthUpgradeSuccess(endpoint_id_1); @@ -964,6 +1031,10 @@ TEST(AnalyticsRecorderTest, StartListeningForIncomingConnectionsWorks) { upgrade_result: WIFI_LAN_MEDIUM_ERROR error_stage: WIFI_LAN_SOCKET_CREATION connection_token: "connection_token" + operation_result < + result_category: CATEGORY_CONNECTIVITY_ERROR + result_code: CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL + > > upgrade_attempt < direction: INCOMING @@ -972,18 +1043,24 @@ TEST(AnalyticsRecorderTest, StartListeningForIncomingConnectionsWorks) { upgrade_result: UPGRADE_RESULT_SUCCESS error_stage: UPGRADE_SUCCESS connection_token: "connection_token" + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > >)pb"); analytics_recorder.Sync(); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } TEST(AnalyticsRecorderTest, SetErrorCodeFieldsCorrectly) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); + analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{WEB_RTC}); @@ -1003,14 +1080,15 @@ TEST(AnalyticsRecorderTest, SetErrorCodeFieldsCorrectly) { connection_token: "connection_token" )pb"); - EXPECT_THAT(event_logger.GetErrorCode(), - Partially(EqualsProto(error_code_proto))); + EXPECT_THAT(event_logger.GetErrorCode(), EqualsProto(error_code_proto)); } TEST(AnalyticsRecorderTest, SetErrorCodeFieldsCorrectlyForUnknownDescription) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); + analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{BLUETOOTH}); @@ -1033,14 +1111,15 @@ TEST(AnalyticsRecorderTest, SetErrorCodeFieldsCorrectlyForUnknownDescription) { connection_token: "connection_token" )pb"); - EXPECT_THAT(event_logger.GetErrorCode(), - Partially(EqualsProto(error_code_proto))); + EXPECT_THAT(event_logger.GetErrorCode(), EqualsProto(error_code_proto)); } TEST(AnalyticsRecorderTest, SetErrorCodeFieldsCorrectlyForCommonError) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); + analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{BLUETOOTH}); @@ -1060,14 +1139,14 @@ TEST(AnalyticsRecorderTest, SetErrorCodeFieldsCorrectlyForCommonError) { connection_token: "connection_token" )pb"); - EXPECT_THAT(event_logger.GetErrorCode(), - Partially(EqualsProto(error_code_proto))); + EXPECT_THAT(event_logger.GetErrorCode(), EqualsProto(error_code_proto)); } TEST(AnalyticsRecorderTest, CheckIfSessionWasLogged) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); // LogSession to count down client_session_done_latch. analytics_recorder.LogSession(); @@ -1083,7 +1162,8 @@ TEST(AnalyticsRecorderTest, ConstructAnalyticsRecorder) { &start_client_session_done_latch); // Call the constructor to count down the session_done_latch. - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); ASSERT_TRUE(start_client_session_done_latch.Await(kDefaultTimeout).result()); std::vector event_types = event_logger.GetLoggedEventTypes(); @@ -1099,7 +1179,8 @@ TEST(AnalyticsRecorderTest, &start_client_session_done_latch); // Call the constructor to count down the start_client_session_done_latch. - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); ASSERT_TRUE(start_client_session_done_latch.Await(kDefaultTimeout).result()); // Log start client session once. @@ -1128,7 +1209,8 @@ TEST(AnalyticsRecorderTest, &start_client_session_done_latch); // Call the constructor to count down the start_client_session_done_latch. - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); ASSERT_TRUE(start_client_session_done_latch.Await(kDefaultTimeout).result()); // Log start client session once. @@ -1165,7 +1247,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -1198,7 +1281,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto1))); + EqualsProto(strategy_session_proto1)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1259,7 +1342,7 @@ TEST(AnalyticsRecorderTest, > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto2)))); + Not(EqualsProto(strategy_session_proto2))); } TEST(AnalyticsRecorderTest, @@ -1268,7 +1351,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartDiscovery(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -1302,7 +1386,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto1))); + EqualsProto(strategy_session_proto1)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1363,7 +1447,7 @@ TEST(AnalyticsRecorderTest, > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto2)))); + Not(EqualsProto(strategy_session_proto2))); } TEST(AnalyticsRecorderTest, ClearcActiveConnectionsAfterSessionWasLogged) { @@ -1374,7 +1458,8 @@ TEST(AnalyticsRecorderTest, ClearcActiveConnectionsAfterSessionWasLogged) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(strategy, mediums); @@ -1402,11 +1487,16 @@ TEST(AnalyticsRecorderTest, ClearcActiveConnectionsAfterSessionWasLogged) { medium: BLUETOOTH disconnection_reason: UNFINISHED connection_token: "connection_token" + safe_disconnection_result: SAFE_DISCONNECTION + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto1))); + EqualsProto(strategy_session_proto1)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1453,11 +1543,16 @@ TEST(AnalyticsRecorderTest, ClearcActiveConnectionsAfterSessionWasLogged) { medium: BLUETOOTH disconnection_reason: UNFINISHED connection_token: "connection_token" + safe_disconnection_result: SAFE_DISCONNECTION + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto2)))); + Not(EqualsProto(strategy_session_proto2))); } TEST(AnalyticsRecorderTest, @@ -1469,7 +1564,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); @@ -1480,8 +1576,9 @@ TEST(AnalyticsRecorderTest, analytics_recorder.OnBandwidthUpgradeStarted( endpoint_id_1, BLUETOOTH, WIFI_LAN, INCOMING, connection_token); // - Error to upgrade. - analytics_recorder.OnBandwidthUpgradeError(endpoint_id, WIFI_LAN_MEDIUM_ERROR, - WIFI_LAN_SOCKET_CREATION); + analytics_recorder.OnBandwidthUpgradeError( + endpoint_id, WIFI_LAN_MEDIUM_ERROR, WIFI_LAN_SOCKET_CREATION, + OperationResultCode::CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL); // - Success to upgrade. analytics_recorder.OnBandwidthUpgradeSuccess(endpoint_id_1); @@ -1517,6 +1614,10 @@ TEST(AnalyticsRecorderTest, upgrade_result: WIFI_LAN_MEDIUM_ERROR error_stage: WIFI_LAN_SOCKET_CREATION connection_token: "connection_token" + operation_result < + result_category: CATEGORY_CONNECTIVITY_ERROR + result_code: CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL + > > upgrade_attempt < direction: INCOMING @@ -1525,6 +1626,10 @@ TEST(AnalyticsRecorderTest, upgrade_result: UPGRADE_RESULT_SUCCESS error_stage: UPGRADE_SUCCESS connection_token: "connection_token" + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > upgrade_attempt { direction: INCOMING @@ -1533,10 +1638,14 @@ TEST(AnalyticsRecorderTest, upgrade_result: UNFINISHED_ERROR error_stage: UPGRADE_UNFINISHED connection_token: "connection_token" + operation_result < + result_category: CATEGORY_DEVICE_STATE_ERROR + result_code: DEVICE_STATE_ERROR_UNFINISHED_UPGRADE_ATTEMPTS + > } >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto1))); + EqualsProto(strategy_session_proto1)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1582,6 +1691,10 @@ TEST(AnalyticsRecorderTest, upgrade_result: WIFI_LAN_MEDIUM_ERROR error_stage: WIFI_LAN_SOCKET_CREATION connection_token: "connection_token" + operation_result < + result_category: CATEGORY_CONNECTIVITY_ERROR + result_code: CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL + > > upgrade_attempt < direction: INCOMING @@ -1590,6 +1703,10 @@ TEST(AnalyticsRecorderTest, upgrade_result: UPGRADE_RESULT_SUCCESS error_stage: UPGRADE_SUCCESS connection_token: "connection_token" + operation_result < + result_category: CATEGORY_SUCCESS + result_code: DETAIL_SUCCESS + > > upgrade_attempt { direction: INCOMING @@ -1598,10 +1715,14 @@ TEST(AnalyticsRecorderTest, upgrade_result: UNFINISHED_ERROR error_stage: UPGRADE_UNFINISHED connection_token: "connection_token" + operation_result < + result_category: CATEGORY_DEVICE_STATE_ERROR + result_code: DEVICE_STATE_ERROR_UNFINISHED_UPGRADE_ATTEMPTS + > } >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto2)))); + Not(EqualsProto(strategy_session_proto2))); } // Test if current_strategy_ is reset by checking if the same strategy would @@ -1613,7 +1734,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLUETOOTH}); @@ -1623,8 +1745,8 @@ TEST(AnalyticsRecorderTest, analytics_recorder.LogSession(); ASSERT_TRUE(client_session_done_latch.Await(kDefaultTimeout).result()); - //// The same strategy session shouldn't be logged again with the same client - //// session. + // The same strategy session shouldn't be logged again with the same client + // session. EXPECT_THAT(event_logger.GetLoggedEventTypes(), Contains(START_STRATEGY_SESSION).Times(1)); @@ -1655,10 +1777,11 @@ TEST(AnalyticsRecorderTest, NotLogSameStrategySessionProtoAfterSessionWasLogged) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); - //// Via OnStartAdvertising, current_strategy_session_is set in - //// UpdateStrategySessionLocked. + // Via OnStartAdvertising, current_strategy_session_is set in + // UpdateStrategySessionLocked. analytics_recorder.OnStartAdvertising(connections::Strategy::kP2pStar, /*mediums=*/{BLE, BLUETOOTH}); analytics_recorder.OnStopAdvertising(); @@ -1684,7 +1807,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1703,7 +1826,7 @@ TEST(AnalyticsRecorderTest, ASSERT_TRUE(new_client_session_done_latch.Await(kDefaultTimeout).result()); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto)))); + Not(EqualsProto(strategy_session_proto))); } // Test if current_advertising_phase_ is reset. @@ -1711,7 +1834,8 @@ TEST(AnalyticsRecorderTest, NotLogDuplicateAdvertisingPhaseAfterSessionWasLogged) { CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartAdvertising( connections::Strategy::kP2pStar, @@ -1738,7 +1862,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto1))); + EqualsProto(strategy_session_proto1)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1780,7 +1904,7 @@ TEST(AnalyticsRecorderTest, > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto2)))); + Not(EqualsProto(strategy_session_proto2))); } // Test if current_discovery_phase_ is reset. @@ -1790,7 +1914,8 @@ TEST(AnalyticsRecorderTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); analytics_recorder.OnStartDiscovery( strategy, {BLUETOOTH}, /*is_extended_advertisement_supported=*/true, @@ -1810,6 +1935,7 @@ TEST(AnalyticsRecorderTest, role: DISCOVERER discovery_phase < medium: BLUETOOTH + discovered_endpoint < medium: BLUETOOTH > discovery_metadata < supports_extended_ble_advertisements: true connected_ap_frequency: 1 @@ -1819,7 +1945,7 @@ TEST(AnalyticsRecorderTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto1))); + EqualsProto(strategy_session_proto1)); // LogStartSession CountDownLatch new_start_client_session_done_latch(1); @@ -1845,6 +1971,7 @@ TEST(AnalyticsRecorderTest, role: DISCOVERER discovery_phase < medium: BLUETOOTH + discovered_endpoint < medium: BLUETOOTH > discovery_metadata < supports_extended_ble_advertisements: true connected_ap_frequency: 1 @@ -1861,7 +1988,7 @@ TEST(AnalyticsRecorderTest, > >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Not(Partially(EqualsProto(strategy_session_proto2)))); + Not(EqualsProto(strategy_session_proto2))); } TEST(AnalyticsRecorderOnConnectionClosedTest, @@ -1870,7 +1997,8 @@ TEST(AnalyticsRecorderOnConnectionClosedTest, CountDownLatch client_session_done_latch(1); FakeEventLogger event_logger(client_session_done_latch); - AnalyticsRecorder analytics_recorder(&event_logger); + AnalyticsRecorder analytics_recorder(&event_logger, + /*no_record_time_millis=*/true); // via OnStartAdvertising, current_strategy_session_ is set in // UpdateStrategySessionLocked. @@ -1899,7 +2027,7 @@ TEST(AnalyticsRecorderOnConnectionClosedTest, >)pb"); EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); // Without calling OnStartAdvertising won't create new // current_strategy_session_. @@ -1913,7 +2041,7 @@ TEST(AnalyticsRecorderOnConnectionClosedTest, // The proto won't change. EXPECT_THAT(event_logger.GetLoggedClientSession(), - Partially(EqualsProto(strategy_session_proto))); + EqualsProto(strategy_session_proto)); } } // namespace diff --git a/connections/implementation/analytics/connection_attempt_metadata_params.h b/connections/implementation/analytics/connection_attempt_metadata_params.h index 65af75ab18..980c033084 100644 --- a/connections/implementation/analytics/connection_attempt_metadata_params.h +++ b/connections/implementation/analytics/connection_attempt_metadata_params.h @@ -37,6 +37,9 @@ struct ConnectionAttemptMetadataParams { int max_wifi_tx_speed = 0; int max_wifi_rx_speed = 0; int channel_width = -1; // -1 as Unknown. + location::nearby::proto::connections::OperationResultCode + operation_result_code = location::nearby::proto::connections:: + OperationResultCode::DETAIL_UNKNOWN; }; } // namespace nearby diff --git a/connections/implementation/base_bwu_handler_test.cc b/connections/implementation/base_bwu_handler_test.cc index 13fe331b2c..b87c8d5bf7 100644 --- a/connections/implementation/base_bwu_handler_test.cc +++ b/connections/implementation/base_bwu_handler_test.cc @@ -14,18 +14,27 @@ #include "connections/implementation/base_bwu_handler.h" +#include +#include #include +#include #include "gtest/gtest.h" #include "absl/strings/string_view.h" +#include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" #include "connections/implementation/service_id_constants.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" namespace nearby { namespace connections { namespace { -// Because BaseBwuHandler is still an abstract class, we need to implement the -// pure virtual functions in order to test BaseBwuHandler's bookkeeping logic. +using ::location::nearby::proto::connections::OperationResultCode; + +// Because BaseBwuHandler is still an abstract class, we need to implement the +// pure virtual functions in order to test BaseBwuHandler's bookkeeping logic. class BwuHandlerImpl : public BaseBwuHandler { public: using Medium = ::location::nearby::proto::connections::Medium; @@ -34,8 +43,8 @@ class BwuHandlerImpl : public BaseBwuHandler { // for every method. struct InputData { ClientProxy* client = nullptr; - absl::optional service_id; - absl::optional endpoint_id; + std::optional service_id; + std::optional endpoint_id; }; BwuHandlerImpl() : BaseBwuHandler(nullptr) {} @@ -52,11 +61,12 @@ class BwuHandlerImpl : public BaseBwuHandler { private: // BwuHandler implementation: - std::unique_ptr CreateUpgradedEndpointChannel( + ErrorOr> + CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) final { - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } Medium GetUpgradeMedium() const final { return Medium::UNKNOWN_MEDIUM; } void OnEndpointDisconnect(ClientProxy* client, diff --git a/connections/implementation/base_pcp_handler.cc b/connections/implementation/base_pcp_handler.cc index 6c1553069f..cdb4f151c8 100644 --- a/connections/implementation/base_pcp_handler.cc +++ b/connections/implementation/base_pcp_handler.cc @@ -85,7 +85,6 @@ namespace nearby { namespace connections { namespace { - constexpr int kEndpointCancelAlarmTimeout = 10; std::string AuthenticationStatusToString(nearby::AuthenticationStatus status) { @@ -98,7 +97,6 @@ std::string AuthenticationStatusToString(nearby::AuthenticationStatus status) { return "failure"; } } - } // namespace using ::location::nearby::connections::ConnectionRequestFrame; @@ -108,11 +106,9 @@ using ::location::nearby::connections::MediumMetadata; using ::location::nearby::connections::OfflineFrame; using ::location::nearby::connections::PresenceDevice; using ::location::nearby::connections::V1Frame; +using ::location::nearby::proto::connections::OperationResultCode; using ::securegcm::UKey2Handshake; -constexpr absl::Duration BasePcpHandler::kConnectionRequestReadTimeout; -constexpr absl::Duration BasePcpHandler::kRejectedConnectionCloseDelay; - BasePcpHandler::BasePcpHandler(Mediums* mediums, EndpointManager* endpoint_manager, EndpointChannelManager* channel_manager, @@ -623,7 +619,9 @@ void BasePcpHandler::OnEncryptionSuccessRunnableV3( connection_info.client, connection_info.medium, remote_device.GetEndpointId(), connection_info.channel.get(), connection_info.is_incoming, connection_info.start_time, - {Status::kEndpointIoError}, connection_info.result.lock().get()); + {Status::kEndpointIoError}, + OperationResultCode::NEARBY_AUTHENTICATION_FAILURE, + connection_info.result.lock().get()); return; } @@ -685,6 +683,7 @@ void BasePcpHandler::OnEncryptionSuccessRunnable( connection_info.client, connection_info.medium, endpoint_id, connection_info.channel.get(), connection_info.is_incoming, connection_info.start_time, {Status::kEndpointIoError}, + OperationResultCode::NEARBY_AUTHENTICATION_FAILURE, connection_info.result.lock().get()); return; } @@ -763,7 +762,7 @@ void BasePcpHandler::OnEncryptionFailureRunnable( ProcessPreConnectionInitiationFailure( info.client, info.medium, endpoint_id, info.channel.get(), info.is_incoming, info.start_time, {Status::kEndpointIoError}, - info.result.lock().get()); + OperationResultCode::NEARBY_ENCRYPTION_FAILURE, info.result.lock().get()); } ConnectionInfo BasePcpHandler::FillConnectionInfo( @@ -862,7 +861,7 @@ Status BasePcpHandler::RequestConnection( ProcessPreConnectionInitiationFailure( client, channel_medium, endpoint_id, channel.get(), /* is_incoming = */ false, start_time, connect_impl_result.status, - result.get()); + connect_impl_result.operation_result_code, result.get()); return; } @@ -878,13 +877,14 @@ Status BasePcpHandler::RequestConnection( Exception write_exception = WriteConnectionRequestFrame( local_device->GetType(), local_device->ToProtoBytes(), connection_info, channel.get()); - if (!write_exception.Ok()) { NEARBY_LOGS(INFO) << "Failed to send connection request: endpoint_id=" << endpoint_id; ProcessPreConnectionInitiationFailure( client, channel_medium, endpoint_id, channel.get(), /* is_incoming = */ false, start_time, {Status::kEndpointIoError}, + client->GetAnalyticsRecorder() + .GetChannelIoErrorResultCodeFromMedium(channel_medium), result.get()); return; } @@ -1005,7 +1005,7 @@ Status BasePcpHandler::RequestConnectionV3( ProcessPreConnectionInitiationFailure( client, channel_medium, endpoint_id, channel.get(), /* is_incoming = */ false, start_time, connect_impl_result.status, - result.get()); + connect_impl_result.operation_result_code, result.get()); return; } @@ -1031,6 +1031,8 @@ Status BasePcpHandler::RequestConnectionV3( ProcessPreConnectionInitiationFailure( client, channel_medium, endpoint_id, channel.get(), /* is_incoming = */ false, start_time, {Status::kEndpointIoError}, + client->GetAnalyticsRecorder() + .GetChannelIoErrorResultCodeFromMedium(channel_medium), result.get()); return; } @@ -1338,7 +1340,8 @@ Exception BasePcpHandler::WriteConnectionRequestFrame( void BasePcpHandler::ProcessPreConnectionInitiationFailure( ClientProxy* client, Medium medium, const std::string& endpoint_id, EndpointChannel* channel, bool is_incoming, absl::Time start_time, - Status status, Future* result) { + Status status, OperationResultCode operation_result_code, + Future* result) { if (channel != nullptr) { channel->Close(); } @@ -1349,7 +1352,7 @@ void BasePcpHandler::ProcessPreConnectionInitiationFailure( } LogConnectionAttemptFailure(client, medium, endpoint_id, is_incoming, - start_time, channel); + start_time, channel, operation_result_code); // result is hold inside a swapper, and saved in PendingConnectionInfo. // PendingConnectionInfo destructor will clear the memory of SettableFuture // shared_ptr for result. @@ -1850,7 +1853,10 @@ Exception BasePcpHandler::OnIncomingConnection( << "with error: " << wrapped_frame.exception(); ProcessPreConnectionInitiationFailure( client, medium, /*endpoint_id=*/"", channel.get(), - /*is_incoming=*/true, start_time, {Status::kError}, nullptr); + /*is_incoming=*/true, start_time, {Status::kError}, + client->GetAnalyticsRecorder().GetChannelIoErrorResultCodeFromMedium( + medium), + nullptr); } return wrapped_frame.GetException(); } @@ -2081,7 +2087,9 @@ void BasePcpHandler::ProcessTieBreakLoss( BasePcpHandler::PendingConnectionInfo* info) { ProcessPreConnectionInitiationFailure( client, info->medium, endpoint_id, info->channel.get(), info->is_incoming, - info->start_time, {Status::kEndpointIoError}, info->result.lock().get()); + info->start_time, {Status::kEndpointIoError}, + OperationResultCode::CLIENT_PROCESS_TIE_BREAK_LOSS, + info->result.lock().get()); ProcessPreConnectionResultFailure(client, endpoint_id, /* should_call_disconnect_endpoint= */ true, DisconnectionReason::IO_ERROR); @@ -2348,8 +2356,8 @@ std::string BasePcpHandler::GetHashedConnectionToken( void BasePcpHandler::LogConnectionAttemptFailure( ClientProxy* client, Medium medium, const std::string& endpoint_id, - bool is_incoming, absl::Time start_time, - EndpointChannel* endpoint_channel) { + bool is_incoming, absl::Time start_time, EndpointChannel* endpoint_channel, + OperationResultCode operation_result_code) { location::nearby::proto::connections::ConnectionAttemptResult result = Cancelled(client, endpoint_id) ? location::nearby::proto::connections::RESULT_CANCELLED @@ -2361,14 +2369,16 @@ void BasePcpHandler::LogConnectionAttemptFailure( client->GetAnalyticsRecorder().BuildConnectionAttemptMetadataParams( endpoint_channel->GetTechnology(), endpoint_channel->GetBand(), endpoint_channel->GetFrequency(), endpoint_channel->GetTryCount()); + connections_attempt_metadata_params->operation_result_code = + operation_result_code; } if (is_incoming) { - client->GetAnalyticsRecorder().OnIncomingConnectionAttempt( + client->GetAnalyticsRecorder().OnIncomingConnectionAttemptWithMetadata( location::nearby::proto::connections::INITIAL, medium, result, SystemClock::ElapsedRealtime() - start_time, /* connection_token= */ "", connections_attempt_metadata_params.get()); } else { - client->GetAnalyticsRecorder().OnOutgoingConnectionAttempt( + client->GetAnalyticsRecorder().OnOutgoingConnectionAttemptWithMetadata( endpoint_id, location::nearby::proto::connections::INITIAL, medium, result, SystemClock::ElapsedRealtime() - start_time, /* connection_token= */ "", connections_attempt_metadata_params.get()); @@ -2388,6 +2398,8 @@ void BasePcpHandler::LogConnectionAttemptSuccess( connection_info.channel->GetBand(), connection_info.channel->GetFrequency(), connection_info.channel->GetTryCount()); + connections_attempt_metadata_params->operation_result_code = + OperationResultCode::DETAIL_SUCCESS; } else { NEARBY_LOGS(ERROR) << "PendingConnectionInfo channel is null for " "LogConnectionAttemptSuccess. Bail out."; @@ -2395,20 +2407,23 @@ void BasePcpHandler::LogConnectionAttemptSuccess( } if (connection_info.is_incoming) { - connection_info.client->GetAnalyticsRecorder().OnIncomingConnectionAttempt( - location::nearby::proto::connections::INITIAL, connection_info.medium, - location::nearby::proto::connections::RESULT_SUCCESS, - SystemClock::ElapsedRealtime() - connection_info.start_time, - connection_info.connection_token, - connections_attempt_metadata_params.get()); + connection_info.client->GetAnalyticsRecorder() + .OnIncomingConnectionAttemptWithMetadata( + location::nearby::proto::connections::INITIAL, + connection_info.medium, + location::nearby::proto::connections::RESULT_SUCCESS, + SystemClock::ElapsedRealtime() - connection_info.start_time, + connection_info.connection_token, + connections_attempt_metadata_params.get()); } else { - connection_info.client->GetAnalyticsRecorder().OnOutgoingConnectionAttempt( - endpoint_id, location::nearby::proto::connections::INITIAL, - connection_info.medium, - location::nearby::proto::connections::RESULT_SUCCESS, - SystemClock::ElapsedRealtime() - connection_info.start_time, - connection_info.connection_token, - connections_attempt_metadata_params.get()); + connection_info.client->GetAnalyticsRecorder() + .OnOutgoingConnectionAttemptWithMetadata( + endpoint_id, location::nearby::proto::connections::INITIAL, + connection_info.medium, + location::nearby::proto::connections::RESULT_SUCCESS, + SystemClock::ElapsedRealtime() - connection_info.start_time, + connection_info.connection_token, + connections_attempt_metadata_params.get()); } } diff --git a/connections/implementation/base_pcp_handler.h b/connections/implementation/base_pcp_handler.h index 042b87683d..ff8bd2145c 100644 --- a/connections/implementation/base_pcp_handler.h +++ b/connections/implementation/base_pcp_handler.h @@ -274,6 +274,9 @@ class BasePcpHandler : public PcpHandler, location::nearby::proto::connections::Medium medium = location::nearby::proto::connections::Medium::UNKNOWN_MEDIUM; Status status = {Status::kError}; + location::nearby::proto::connections::OperationResultCode + operation_result_code = location::nearby::proto::connections:: + OperationResultCode::DETAIL_UNKNOWN; std::unique_ptr endpoint_channel; }; @@ -567,7 +570,10 @@ class BasePcpHandler : public PcpHandler, void ProcessPreConnectionInitiationFailure( ClientProxy* client, Medium medium, const std::string& endpoint_id, EndpointChannel* channel, bool is_incoming, absl::Time start_time, - Status status, Future* result); + Status status, + location::nearby::proto::connections::OperationResultCode + operation_result_code, + Future* result); void ProcessPreConnectionResultFailure(ClientProxy* client, const std::string& endpoint_id, bool should_call_disconnect_endpoint, @@ -593,11 +599,12 @@ class BasePcpHandler : public PcpHandler, // array. std::string GetHashedConnectionToken(const ByteArray& token_bytes); - static void LogConnectionAttemptFailure(ClientProxy* client, Medium medium, - const std::string& endpoint_id, - bool is_incoming, - absl::Time start_time, - EndpointChannel* endpoint_channel); + static void LogConnectionAttemptFailure( + ClientProxy* client, Medium medium, const std::string& endpoint_id, + bool is_incoming, absl::Time start_time, + EndpointChannel* endpoint_channel, + location::nearby::proto::connections::OperationResultCode + operation_result_code); static void LogConnectionAttemptSuccess( const std::string& endpoint_id, diff --git a/connections/implementation/bluetooth_bwu_handler.cc b/connections/implementation/bluetooth_bwu_handler.cc index 74e723125e..24a49e5351 100644 --- a/connections/implementation/bluetooth_bwu_handler.cc +++ b/connections/implementation/bluetooth_bwu_handler.cc @@ -14,13 +14,21 @@ #include "connections/implementation/bluetooth_bwu_handler.h" +#include #include #include #include "absl/functional/bind_front.h" +#include "connections/implementation/base_bwu_handler.h" #include "connections/implementation/bluetooth_endpoint_channel.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" +#include "connections/implementation/mediums/mediums.h" #include "connections/implementation/offline_frames.h" +#include "internal/platform/bluetooth_adapter.h" +#include "internal/platform/bluetooth_classic.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" #include "internal/platform/logging.h" // Manages the Bluetooth-specific methods needed to upgrade an {@link @@ -29,6 +37,11 @@ namespace nearby { namespace connections { +namespace { +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace + +// TODO(edwinwu): Add exact OperationResultCode for BluetoothBwuHandler. BluetoothBwuHandler::BluetoothBwuHandler( Mediums& mediums, IncomingConnectionCallback incoming_connection_callback) : BaseBwuHandler(std::move(incoming_connection_callback)), @@ -37,7 +50,7 @@ BluetoothBwuHandler::BluetoothBwuHandler( // Called by BWU target. Retrieves a new medium info from incoming message, // and establishes connection over BT using this info. // Returns a channel ready to exchange data or nullptr on error. -std::unique_ptr +ErrorOr> BluetoothBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { @@ -47,7 +60,7 @@ BluetoothBwuHandler::CreateUpgradedEndpointChannel( !bluetooth_credentials.has_mac_address()) { NEARBY_LOGS(ERROR) << "BluetoothBwuHandler failed to parse UpgradePathInfo."; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } const std::string& service_name = bluetooth_credentials.service_name(); @@ -64,7 +77,7 @@ BluetoothBwuHandler::CreateUpgradedEndpointChannel( << "BluetoothBwuHandler failed to derive a valid Bluetooth device " "from the MAC address (" << mac_address << ") for endpoint " << endpoint_id; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } BluetoothSocket socket = bluetooth_medium_.Connect( @@ -74,7 +87,7 @@ BluetoothBwuHandler::CreateUpgradedEndpointChannel( << "BluetoothBwuHandler failed to connect to the Bluetooth device (" << service_name << ", " << mac_address << ") for endpoint " << endpoint_id << " and service ID " << service_id; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } NEARBY_VLOG(1) @@ -91,11 +104,11 @@ BluetoothBwuHandler::CreateUpgradedEndpointChannel( << service_name << ", " << mac_address << ") for endpoint " << endpoint_id << " and service ID " << service_id; socket.Close(); - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } client->SetBluetoothMacAddress(endpoint_id, mac_address); - return channel; + return {std::move(channel)}; } ByteArray BluetoothBwuHandler::HandleInitializeUpgradedMediumForEndpoint( @@ -146,7 +159,7 @@ void BluetoothBwuHandler::HandleRevertInitiatorStateForService( void BluetoothBwuHandler::OnIncomingBluetoothConnection( ClientProxy* client, const std::string& upgrade_service_id, BluetoothSocket socket) { - auto channel = absl::make_unique( + auto channel = std::make_unique( upgrade_service_id, /*channel_name=*/upgrade_service_id, socket); std::unique_ptr connection{ new IncomingSocketConnection{ diff --git a/connections/implementation/bluetooth_bwu_handler.h b/connections/implementation/bluetooth_bwu_handler.h index 6563eebf8f..46466ee24b 100644 --- a/connections/implementation/bluetooth_bwu_handler.h +++ b/connections/implementation/bluetooth_bwu_handler.h @@ -15,14 +15,20 @@ #ifndef CORE_INTERNAL_BLUETOOTH_BWU_HANDLER_H_ #define CORE_INTERNAL_BLUETOOTH_BWU_HANDLER_H_ +#include #include +#include #include "connections/implementation/base_bwu_handler.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" +#include "connections/implementation/mediums/bluetooth_classic.h" +#include "connections/implementation/mediums/bluetooth_radio.h" #include "connections/implementation/mediums/mediums.h" -#include "connections/implementation/mediums/utils.h" +#include "connections/medium_selector.h" #include "internal/platform/bluetooth_classic.h" -#include "internal/platform/count_down_latch.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" namespace nearby { namespace connections { @@ -51,10 +57,11 @@ class BluetoothBwuHandler : public BaseBwuHandler { }; // BwuHandler implementation: - std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) final; + ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) final; Medium GetUpgradeMedium() const final { return Medium::BLUETOOTH; } void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) final {} diff --git a/connections/implementation/bluetooth_bwu_test.cc b/connections/implementation/bluetooth_bwu_test.cc index 6b0b989740..5e1631c5d6 100644 --- a/connections/implementation/bluetooth_bwu_test.cc +++ b/connections/implementation/bluetooth_bwu_test.cc @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include "gtest/gtest.h" #include "absl/time/time.h" @@ -25,6 +26,7 @@ #include "internal/platform/byte_array.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/exception.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" #include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" @@ -35,6 +37,7 @@ namespace connections { namespace { using ::location::nearby::connections::OfflineFrame; +using ::location::nearby::proto::connections::OperationResultCode; constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); } // namespace @@ -101,17 +104,22 @@ TEST_F(BluetoothBwuTest, SoftAPBWUInit_STACreateEndpointChannel) { auto bwu_frame = upgrade_frame.result().v1().bandwidth_upgrade_negotiation(); - std::unique_ptr new_channel = + ErrorOr> result = handler_2->CreateUpgradedEndpointChannel(&client_2, /*service_id=*/"A", /*endpoint_id=*/"1", bwu_frame.upgrade_path_info()); if (!FeatureFlags::GetInstance().GetFlags().enable_cancellation_flag) { + ASSERT_TRUE(result.has_value()); + std::unique_ptr new_channel = std::move(result.value()); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); EXPECT_EQ(new_channel->GetMedium(), location::nearby::proto::connections::Medium::BLUETOOTH); } else { + EXPECT_FALSE(result.has_value()); + EXPECT_TRUE(result.has_error()); + EXPECT_EQ(result.error().operation_result_code(), + OperationResultCode::DETAIL_UNKNOWN); accept_latch.CountDown(); - EXPECT_EQ(new_channel, nullptr); } EXPECT_FALSE(mediums_2.GetBluetoothClassic().GetMacAddress().empty()); handler_2->RevertResponderState(/*service_id=*/"A"); diff --git a/connections/implementation/bwu_handler.h b/connections/implementation/bwu_handler.h index de1ee7e04d..0deb84bc7c 100644 --- a/connections/implementation/bwu_handler.h +++ b/connections/implementation/bwu_handler.h @@ -15,14 +15,15 @@ #ifndef CORE_INTERNAL_BWU_HANDLER_H_ #define CORE_INTERNAL_BWU_HANDLER_H_ -#include +#include #include #include "absl/functional/any_invocable.h" #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel.h" #include "connections/implementation/offline_frames.h" -#include "internal/platform/count_down_latch.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" namespace nearby { namespace connections { @@ -81,14 +82,16 @@ class BwuHandler { // that hasn't already been done) using the UpgradePathInfo sent by the // Initiator, and returns a new EndpointChannel for the upgraded medium. // @BwuHandlerThread - virtual std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) = 0; + virtual ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) = 0; // Returns the upgrade medium of the BwuHandler. // @BwuHandlerThread - virtual Medium GetUpgradeMedium() const = 0; + virtual location::nearby::proto::connections::Medium GetUpgradeMedium() + const = 0; virtual void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) = 0; diff --git a/connections/implementation/bwu_manager.cc b/connections/implementation/bwu_manager.cc index 28f37f2cef..c77c96a8ad 100644 --- a/connections/implementation/bwu_manager.cc +++ b/connections/implementation/bwu_manager.cc @@ -31,6 +31,7 @@ #include "connections/implementation/endpoint_channel.h" #include "connections/implementation/endpoint_channel_manager.h" #include "connections/implementation/endpoint_manager.h" +#include "connections/implementation/mediums/mediums.h" #include "connections/implementation/offline_frames.h" #include "connections/implementation/service_id_constants.h" #ifdef NO_WEBRTC @@ -45,6 +46,7 @@ #include "internal/platform/byte_array.h" #include "internal/platform/cancelable_alarm.h" #include "internal/platform/count_down_latch.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" #include "internal/platform/implementation/system_clock.h" #include "internal/platform/logging.h" @@ -54,13 +56,17 @@ namespace nearby { namespace connections { +namespace { using ::location::nearby::connections::BandwidthUpgradeNegotiationFrame; using ::location::nearby::connections::OfflineFrame; using ::location::nearby::connections::V1Frame; +using ::location::nearby::proto::connections::BandwidthUpgradeErrorStage; +using ::location::nearby::proto::connections::BandwidthUpgradeResult; +using ::location::nearby::proto::connections::ConnectionAttemptResult; +using ::location::nearby::proto::connections::ConnectionAttemptType; using ::location::nearby::proto::connections::DisconnectionReason; - -// Required for C++ 14 support in Chrome -constexpr absl::Duration BwuManager::kReadClientIntroductionFrameTimeout; +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace BwuManager::BwuManager( Mediums& mediums, EndpointManager& endpoint_manager, @@ -258,8 +264,9 @@ void BwuManager::InitiateBwuForEndpoint(ClientProxy* client, << endpoint_id << " because it couldn't find an existing EndpointChannel for it."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::CHANNEL_ERROR, - location::nearby::proto::connections::NETWORK_AVAILABLE); + endpoint_id, BandwidthUpgradeResult::CHANNEL_ERROR, + BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, + OperationResultCode::NEARBY_GENERIC_OLD_ENDPOINT_CHANNEL_NULL); return; } @@ -277,6 +284,10 @@ void BwuManager::InitiateBwuForEndpoint(ClientProxy* client, << " because it is already connected over medium " << location::nearby::proto::connections::Medium_Name( proposed_medium); + client->GetAnalyticsRecorder().OnBandwidthUpgradeError( + endpoint_id, BandwidthUpgradeResult::ALREADY_ON_MEDIUM_ERROR, + BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, + OperationResultCode::MEDIUM_UNAVAILABLE_UPGRADE_ON_SAME_MEDIUM); return; } @@ -296,10 +307,16 @@ void BwuManager::InitiateBwuForEndpoint(ClientProxy* client, UpgradePathInfo info; info.set_medium(parser::MediumToUpgradePathInfoMedium(proposed_medium)); - ProcessUpgradeFailureEvent(client, endpoint_id, info); + ProcessUpgradeFailureEvent( + client, endpoint_id, info, + BandwidthUpgradeResult::REMOTE_CONNECTION_ERROR, + /*record_analytic=*/false, + OperationResultCode::CONNECTIVITY_GENERIC_WRITING_CHANNEL_IO_ERROR); + client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_IO_ERROR, - location::nearby::proto::connections::NETWORK_AVAILABLE); + endpoint_id, BandwidthUpgradeResult::RESULT_IO_ERROR, + BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, + OperationResultCode::CONNECTIVITY_GENERIC_WRITING_CHANNEL_IO_ERROR); return; } if (!channel->Write(bytes).Ok()) { @@ -503,8 +520,11 @@ void BwuManager::OnBwuNegotiationFrame(ClientProxy* client, frame.upgrade_path_info()); break; case BwuNegotiationFrame::UPGRADE_FAILURE: - ProcessUpgradeFailureEvent(client, endpoint_id, - frame.upgrade_path_info()); + ProcessUpgradeFailureEvent( + client, endpoint_id, frame.upgrade_path_info(), + BandwidthUpgradeResult::REMOTE_CONNECTION_ERROR, + /* record_analytic= */ true, + OperationResultCode::NEARBY_GENERIC_REMOTE_UPGRADE_FAILURE); break; case BwuNegotiationFrame::LAST_WRITE_TO_PRIOR_CHANNEL: ProcessLastWriteToPriorChannelEvent(client, endpoint_id); @@ -539,7 +559,8 @@ void BwuManager::OnIncomingConnection( connection->socket->Close(); AttemptToRecordBandwidthUpgradeErrorForUnknownEndpoint( location::nearby::proto::connections::MEDIUM_ERROR, - location::nearby::proto::connections::SOCKET_CREATION); + BandwidthUpgradeErrorStage::SOCKET_CREATION, + OperationResultCode::NEARBY_GENERIC_NEW_ENDPOINT_CHANNEL_NULL); return; } @@ -603,8 +624,10 @@ void BwuManager::OnIncomingConnection( client->GetAnalyticsRecorder().BuildConnectionAttemptMetadataParams( channel->GetTechnology(), channel->GetBand(), channel->GetFrequency(), channel->GetTryCount()); + connections_attempt_metadata_params->operation_result_code = + OperationResultCode::DETAIL_SUCCESS; } - client->GetAnalyticsRecorder().OnIncomingConnectionAttempt( + client->GetAnalyticsRecorder().OnIncomingConnectionAttemptWithMetadata( location::nearby::proto::connections::UPGRADE, channel->GetMedium(), location::nearby::proto::connections::RESULT_SUCCESS, SystemClock::ElapsedRealtime() - connection_attempt_start_time, @@ -653,8 +676,9 @@ void BwuManager::RunUpgradeProtocol( << " when registering the new EndpointChannel, short-circuiting the " "upgrade protocol."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::CHANNEL_ERROR, - location::nearby::proto::connections::PRIOR_ENDPOINT_CHANNEL); + endpoint_id, BandwidthUpgradeResult::CHANNEL_ERROR, + BandwidthUpgradeErrorStage::PRIOR_ENDPOINT_CHANNEL, + OperationResultCode::NEARBY_GENERIC_OLD_ENDPOINT_CHANNEL_NULL); return; } channel_manager_->ReplaceChannelForEndpoint( @@ -670,8 +694,9 @@ void BwuManager::RunUpgradeProtocol( "endpoint " << endpoint_id << ", short-circuiting the upgrade protocol."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_IO_ERROR, - location::nearby::proto::connections::LAST_WRITE_TO_PRIOR_CHANNEL); + endpoint_id, BandwidthUpgradeResult::RESULT_IO_ERROR, + BandwidthUpgradeErrorStage::LAST_WRITE_TO_PRIOR_CHANNEL, + OperationResultCode::CONNECTIVITY_GENERIC_WRITING_CHANNEL_IO_ERROR); return; } NEARBY_VLOG(1) << "BwuManager successfully wrote " @@ -770,10 +795,12 @@ void BwuManager::ProcessBwuPathAvailableEvent( client->GetConnectionToken(endpoint_id)); absl::Time connection_attempt_start_time = SystemClock::ElapsedRealtime(); - auto channel = ProcessBwuPathAvailableEventInternal(client, endpoint_id, - upgrade_path_info); - location::nearby::proto::connections::ConnectionAttemptResult - connection_attempt_result; + ErrorOr> result = + ProcessBwuPathAvailableEventInternal(client, endpoint_id, + upgrade_path_info); + std::unique_ptr channel = + result.has_value() ? std::move(result.value()) : nullptr; + ConnectionAttemptResult connection_attempt_result; if (channel != nullptr) { connection_attempt_result = location::nearby::proto::connections::RESULT_SUCCESS; @@ -781,27 +808,36 @@ void BwuManager::ProcessBwuPathAvailableEvent( connection_attempt_result = location::nearby::proto::connections::RESULT_CANCELLED; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_REMOTE_ERROR, - location::nearby::proto::connections::UPGRADE_CANCEL); + endpoint_id, BandwidthUpgradeResult::RESULT_REMOTE_ERROR, + BandwidthUpgradeErrorStage::UPGRADE_CANCEL, + OperationResultCode::CLIENT_CANCELLATION_UPGRADE_CANCELED_BY_REMOTE); } else { connection_attempt_result = location::nearby::proto::connections::RESULT_ERROR; } - std::unique_ptr - connections_attempt_metadata_params; if (channel != nullptr) { - connections_attempt_metadata_params = - client->GetAnalyticsRecorder().BuildConnectionAttemptMetadataParams( - channel->GetTechnology(), channel->GetBand(), - channel->GetFrequency(), channel->GetTryCount()); + std::unique_ptr + connections_attempt_metadata_params = + client->GetAnalyticsRecorder().BuildConnectionAttemptMetadataParams( + channel->GetTechnology(), channel->GetBand(), + channel->GetFrequency(), channel->GetTryCount()); + connections_attempt_metadata_params->operation_result_code = + OperationResultCode::DETAIL_SUCCESS; + client->GetAnalyticsRecorder().OnOutgoingConnectionAttemptWithMetadata( + endpoint_id, ConnectionAttemptType::UPGRADE, upgrade_medium, + connection_attempt_result, + SystemClock::ElapsedRealtime() - connection_attempt_start_time, + client->GetConnectionToken(endpoint_id), + connections_attempt_metadata_params.get()); + } else { + client->GetAnalyticsRecorder().OnOutgoingConnectionAttempt( + endpoint_id, ConnectionAttemptType::UPGRADE, upgrade_medium, + connection_attempt_result, + SystemClock::ElapsedRealtime() - connection_attempt_start_time, + client->GetConnectionToken(endpoint_id), + result.error().operation_result_code().value()); } - client->GetAnalyticsRecorder().OnOutgoingConnectionAttempt( - endpoint_id, location::nearby::proto::connections::UPGRADE, - upgrade_medium, connection_attempt_result, - SystemClock::ElapsedRealtime() - connection_attempt_start_time, - client->GetConnectionToken(endpoint_id), - connections_attempt_metadata_params.get()); if (channel == nullptr) { NEARBY_LOGS(INFO) << "Failed to get new channel."; @@ -814,9 +850,9 @@ void BwuManager::ProcessBwuPathAvailableEvent( !upgrade_path_info.supports_disabling_encryption()); } -std::unique_ptr +ErrorOr> BwuManager::ProcessBwuPathAvailableEventInternal( - ClientProxy* client, const string& endpoint_id, + ClientProxy* client, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { Medium medium = parser::UpgradePathInfoMediumToMedium(upgrade_path_info.medium()); @@ -826,7 +862,7 @@ BwuManager::ProcessBwuPathAvailableEventInternal( << endpoint_id << " medium " << location::nearby::proto::connections::Medium_Name(medium) << ". Upgrade medium not yet set for endpoint."; - return nullptr; + return {Error(OperationResultCode::NEARBY_UPGRADE_PATH_ON_WRONG_MEDIUM)}; } BwuHandler* handler = GetHandlerForMedium(medium); @@ -836,7 +872,8 @@ BwuManager::ProcessBwuPathAvailableEventInternal( << endpoint_id << " medium " << location::nearby::proto::connections::Medium_Name(medium) << ". No handler for medium."; - return nullptr; + // TBD(edwinwu): Add a new operation result code for this. + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } NEARBY_LOGS(INFO) << "ProcessBwuPathAvailableEventInternal for " @@ -857,23 +894,26 @@ BwuManager::ProcessBwuPathAvailableEventInternal( << endpoint_id << " medium " << location::nearby::proto::connections::Medium_Name(medium) << ". Old endpoint channel is missing."; - return nullptr; + // TBD(edwinwu): Add a new operation result code for this. + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } service_id = old_channel->GetServiceId(); } - std::unique_ptr new_channel = + ErrorOr> result = handler->CreateUpgradedEndpointChannel(client, service_id, endpoint_id, upgrade_path_info); - if (!new_channel) { + if (result.has_error() || !result.has_value()) { NEARBY_LOGS(ERROR) << "BwuManager failed to create an endpoint " "channel to endpoint" << endpoint_id << ", aborting upgrade."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_IO_ERROR, - location::nearby::proto::connections::SOCKET_CREATION); - return nullptr; + endpoint_id, BandwidthUpgradeResult::RESULT_IO_ERROR, + BandwidthUpgradeErrorStage::SOCKET_CREATION, + result.error().operation_result_code().value()); + return result; } + std::unique_ptr new_channel = std::move(result.value()); // Write the requisite BANDWIDTH_UPGRADE_NEGOTIATION.CLIENT_INTRODUCTION as // the first OfflineFrame on this new EndpointChannel. @@ -891,9 +931,13 @@ BwuManager::ProcessBwuPathAvailableEventInternal( "OfflineFrame to newly-created EndpointChannel " << new_channel->GetName() << ", aborting upgrade."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_IO_ERROR, - location::nearby::proto::connections::CLIENT_INTRODUCTION); - return {}; + endpoint_id, BandwidthUpgradeResult::RESULT_IO_ERROR, + BandwidthUpgradeErrorStage::CLIENT_INTRODUCTION, + OperationResultCode:: + NEARBY_GENERIC_READ_CLIENT_INTRODUCTION_ACK_FORMAT_ERROR); + return { + Error(OperationResultCode:: + NEARBY_GENERIC_READ_CLIENT_INTRODUCTION_ACK_FORMAT_ERROR)}; } if (upgrade_path_info.supports_client_introduction_ack()) { @@ -906,8 +950,9 @@ BwuManager::ProcessBwuPathAvailableEventInternal( "BWU_NEGOTIATION.CLIENT_INTRODUCTION_ACK " "OfflineFrame to newly-created EndpointChannel " << new_channel->GetName() << ", aborting upgrade."; - - return {}; + return {Error( + OperationResultCode:: + NEARBY_GENERIC_READ_CLIENT_INTRODUCTION_ACK_FRAME_TYPE_ERROR)}; } } @@ -919,7 +964,7 @@ BwuManager::ProcessBwuPathAvailableEventInternal( // Set the AnalyticsRecorder so that the future closure of this // EndpointChannel will be recorded. - return new_channel; + return {std::move(new_channel)}; } void BwuManager::RunUpgradeFailedProtocol( @@ -942,8 +987,9 @@ void BwuManager::RunUpgradeFailedProtocol( << " when sending an upgrade failure frame, short-circuiting the " "upgrade protocol."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::CHANNEL_ERROR, - location::nearby::proto::connections::NETWORK_AVAILABLE); + endpoint_id, BandwidthUpgradeResult::CHANNEL_ERROR, + BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, + OperationResultCode::NEARBY_GENERIC_OLD_ENDPOINT_CHANNEL_NULL); return; } @@ -956,8 +1002,9 @@ void BwuManager::RunUpgradeFailedProtocol( "OfflineFrame to endpoint " << endpoint_id << ", short-circuiting the upgrade protocol."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_IO_ERROR, - location::nearby::proto::connections::NETWORK_AVAILABLE); + endpoint_id, BandwidthUpgradeResult::RESULT_IO_ERROR, + BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, + OperationResultCode::CONNECTIVITY_GENERIC_WRITING_CHANNEL_IO_ERROR); return; } @@ -1102,8 +1149,9 @@ void BwuManager::ProcessLastWriteToPriorChannelEvent( << endpoint_id << ", short-circuiting the upgrade protocol."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::RESULT_IO_ERROR, - location::nearby::proto::connections::SAFE_TO_CLOSE_PRIOR_CHANNEL); + endpoint_id, BandwidthUpgradeResult::RESULT_IO_ERROR, + BandwidthUpgradeErrorStage::SAFE_TO_CLOSE_PRIOR_CHANNEL, + OperationResultCode::CONNECTIVITY_GENERIC_WRITING_CHANNEL_IO_ERROR); return; } NEARBY_VLOG(1) << "BwuManager successfully wrote " @@ -1209,7 +1257,8 @@ void BwuManager::ProcessSafeToClosePriorChannelEvent( void BwuManager::ProcessUpgradeFailureEvent( ClientProxy* client, const std::string& endpoint_id, - const UpgradePathInfo& upgrade_info) { + const UpgradePathInfo& upgrade_info, BandwidthUpgradeResult result, + bool record_analytic, OperationResultCode operation_result_code) { NEARBY_LOGS(INFO) << "ProcessUpgradeFailureEvent for endpoint " << endpoint_id << " from medium: " << location::nearby::proto::connections::Medium_Name( @@ -1235,8 +1284,8 @@ void BwuManager::ProcessUpgradeFailureEvent( << " because we have other connected endpoints and can't try a new " "upgrade medium."; client->GetAnalyticsRecorder().OnBandwidthUpgradeError( - endpoint_id, location::nearby::proto::connections::CHANNEL_ERROR, - location::nearby::proto::connections::NETWORK_AVAILABLE); + endpoint_id, BandwidthUpgradeResult::CHANNEL_ERROR, + BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, operation_result_code); return; } @@ -1252,6 +1301,12 @@ void BwuManager::ProcessUpgradeFailureEvent( RevertBwuMediumForEndpoint(upgrade_service_id, endpoint_id); } + if (record_analytic) { + client->GetAnalyticsRecorder().OnBandwidthUpgradeError( + endpoint_id, result, BandwidthUpgradeErrorStage::NETWORK_AVAILABLE, + operation_result_code); + } + // Loop through the ordered list of upgrade mediums. One by one, remove the // top element until we get to the medium we last attempted to upgrade to. The // remainder of the list will contain the mediums we haven't attempted yet. @@ -1429,9 +1484,8 @@ void BwuManager::RetryUpgradesAfterDelay(ClientProxy* client, } void BwuManager::AttemptToRecordBandwidthUpgradeErrorForUnknownEndpoint( - location::nearby::proto::connections::BandwidthUpgradeResult result, - location::nearby::proto::connections::BandwidthUpgradeErrorStage - error_stage) { + BandwidthUpgradeResult result, BandwidthUpgradeErrorStage error_stage, + OperationResultCode operation_result_code) { if (in_progress_upgrades_.size() == 1) { auto it = in_progress_upgrades_.begin(); std::string endpoint_id = it->first; @@ -1441,28 +1495,20 @@ void BwuManager::AttemptToRecordBandwidthUpgradeErrorForUnknownEndpoint( // them if they want to repeatedly attempt to connect or if they want to // give up and have us try a different medium. This isn't a decision we can // make for them. - client->GetAnalyticsRecorder().OnBandwidthUpgradeError(endpoint_id, result, - error_stage); - NEARBY_LOGS(INFO) - << "BwuManager got error " - << location::nearby::proto::connections::BandwidthUpgradeResult_Name( - result) - << " at stage " - << location::nearby::proto::connections:: - BandwidthUpgradeErrorStage_Name(error_stage) - << " when upgrading endpoint " << endpoint_id; + client->GetAnalyticsRecorder().OnBandwidthUpgradeError( + endpoint_id, result, error_stage, operation_result_code); + NEARBY_LOGS(INFO) << "BwuManager got error " + << BandwidthUpgradeResult_Name(result) << " at stage " + << BandwidthUpgradeErrorStage_Name(error_stage) + << " when upgrading endpoint " << endpoint_id; } // Otherwise, we have no way of knowing which endpoint was trying to connect // to us :( - NEARBY_LOGS(INFO) - << "BwuManager got error " - << location::nearby::proto::connections::BandwidthUpgradeResult_Name( - result) - << " at stage " - << location::nearby::proto::connections::BandwidthUpgradeErrorStage_Name( - error_stage) - << ", but we don't know which endpoint was trying to " - "connect to us, so skipping analytics for his error."; + NEARBY_LOGS(INFO) << "BwuManager got error " + << BandwidthUpgradeResult_Name(result) << " at stage " + << BandwidthUpgradeErrorStage_Name(error_stage) + << ", but we don't know which endpoint was trying to " + "connect to us, so skipping analytics for his error."; } absl::Duration BwuManager::CalculateNextRetryDelay( diff --git a/connections/implementation/bwu_manager.h b/connections/implementation/bwu_manager.h index c13d320837..56f40870f6 100644 --- a/connections/implementation/bwu_manager.h +++ b/connections/implementation/bwu_manager.h @@ -15,7 +15,6 @@ #ifndef CORE_INTERNAL_BWU_MANAGER_H_ #define CORE_INTERNAL_BWU_MANAGER_H_ -#include #include #include #include @@ -26,9 +25,17 @@ #include "absl/time/time.h" #include "connections/implementation/bwu_handler.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" +#include "connections/implementation/endpoint_channel_manager.h" #include "connections/implementation/endpoint_manager.h" #include "connections/implementation/mediums/mediums.h" +#include "connections/medium_selector.h" +#include "internal/platform/cancelable_alarm.h" +#include "internal/platform/count_down_latch.h" +#include "internal/platform/expected.h" +#include "internal/platform/runnable.h" #include "internal/platform/scheduled_executor.h" +#include "internal/platform/single_thread_executor.h" namespace nearby { namespace connections { @@ -167,7 +174,8 @@ class BwuManager : public EndpointManager::FrameProcessor { void ProcessBwuPathAvailableEvent(ClientProxy* client, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info); - std::unique_ptr ProcessBwuPathAvailableEventInternal( + ErrorOr> + ProcessBwuPathAvailableEventInternal( ClientProxy* client, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info); void ProcessLastWriteToPriorChannelEvent(ClientProxy* client, @@ -181,9 +189,13 @@ class BwuManager : public EndpointManager::FrameProcessor { void ProcessEndpointDisconnection(ClientProxy* client, const std::string& endpoint_id, CountDownLatch* barrier); - void ProcessUpgradeFailureEvent(ClientProxy* client, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_info); + void ProcessUpgradeFailureEvent( + ClientProxy* client, const std::string& endpoint_id, + const UpgradePathInfo& upgrade_info, + location::nearby::proto::connections::BandwidthUpgradeResult result, + bool record_analytic, + location::nearby::proto::connections::OperationResultCode + operation_result_code); void CancelRetryUpgradeAlarm(const std::string& endpoint_id); void CancelAllRetryUpgradeAlarms(); void TryNextBestUpgradeMediums(ClientProxy* client, @@ -195,7 +207,9 @@ class BwuManager : public EndpointManager::FrameProcessor { void AttemptToRecordBandwidthUpgradeErrorForUnknownEndpoint( location::nearby::proto::connections::BandwidthUpgradeResult result, location::nearby::proto::connections::BandwidthUpgradeErrorStage - error_stage); + error_stage, + location::nearby::proto::connections::OperationResultCode + operation_result_code); bool is_single_threaded_for_testing_ = false; diff --git a/connections/implementation/client_proxy.cc b/connections/implementation/client_proxy.cc index 174a1a4044..30159c0341 100644 --- a/connections/implementation/client_proxy.cc +++ b/connections/implementation/client_proxy.cc @@ -66,6 +66,7 @@ namespace nearby { namespace connections { + namespace { using ::location::nearby::connections::OsInfo; @@ -79,13 +80,8 @@ bool IsFeatureUseStableEndpointIdEnabled() { connections::config_package_nearby::nearby_connections_feature:: kUseStableEndpointId); } - } // namespace -// The definition is necessary before C++17. -constexpr absl::Duration - ClientProxy::kHighPowerAdvertisementEndpointIdCacheTimeout; - ClientProxy::ClientProxy(::nearby::analytics::EventLogger* event_logger) : client_id_(Prng().NextInt64()) { NEARBY_LOGS(INFO) << "ClientProxy ctor event_logger=" << event_logger; @@ -558,8 +554,9 @@ void ClientProxy::OnBandwidthChanged(const std::string& endpoint_id, NEARBY_LOGS(INFO) << "ClientProxy [BandwidthChanged]: id=" << endpoint_id; MutexLock lock(&mutex_); - const ConnectionPair* item = LookupConnection(endpoint_id); + ConnectionPair* item = LookupConnection(endpoint_id); if (item != nullptr) { + item->first.connected_medium = new_medium; item->first.connection_listener.bandwidth_changed_cb(endpoint_id, new_medium); NEARBY_LOGS(INFO) << "ClientProxy [reporting onBandwidthChanged]: client=" @@ -600,6 +597,17 @@ bool ClientProxy::ConnectionStatusMatches(const std::string& endpoint_id, return false; } +Medium ClientProxy::GetConnectedMedium(const std::string& endpoint_id) const { + MutexLock lock(&mutex_); + + const ConnectionPair* item = LookupConnection(endpoint_id); + if (item != nullptr) { + return item->first.connected_medium; + } + + return Medium::UNKNOWN_MEDIUM; +} + BooleanMediumSelector ClientProxy::GetUpgradeMediums( const std::string& endpoint_id) const { MutexLock lock(&mutex_); diff --git a/connections/implementation/client_proxy.h b/connections/implementation/client_proxy.h index 881c160577..0cf143fd36 100644 --- a/connections/implementation/client_proxy.h +++ b/connections/implementation/client_proxy.h @@ -179,6 +179,9 @@ class ClientProxy final { // ConnectionListener.disconnected_cb() callback. void OnDisconnected(const std::string& endpoint_id, bool notify); + // Returns the medium we're currently connected to the endpoint over, or + // UNKNOWN if we don't know or don't have a connection. + Medium GetConnectedMedium(const std::string& endpoint_id) const; // Returns all mediums eligible for upgrade. BooleanMediumSelector GetUpgradeMediums(const std::string& endpoint_id) const; // Returns if this endpoint support 5G for WIFI. @@ -363,6 +366,7 @@ class ClientProxy final { kConnected = 1 << 4, }; bool is_incoming{false}; + Medium connected_medium{Medium::UNKNOWN_MEDIUM}; Status status{kPending}; ConnectionListener connection_listener; ConnectionOptions connection_options; diff --git a/connections/implementation/endpoint_manager.cc b/connections/implementation/endpoint_manager.cc index 5942072008..26236aaf36 100644 --- a/connections/implementation/endpoint_manager.cc +++ b/connections/implementation/endpoint_manager.cc @@ -30,7 +30,6 @@ #include "connections/implementation/endpoint_channel.h" #include "connections/implementation/endpoint_channel_manager.h" #include "connections/implementation/offline_frames.h" -#include "connections/implementation/payload_manager.h" #include "connections/implementation/proto/offline_wire_formats.pb.h" #include "connections/implementation/service_id_constants.h" #include "connections/listeners.h" @@ -55,10 +54,10 @@ namespace connections { namespace { using ::location::nearby::analytics::proto::ConnectionsLog; using ::location::nearby::connections::OfflineFrame; +using ::location::nearby::connections::PayloadTransferFrame; using ::location::nearby::connections::V1Frame; +using ::location::nearby::proto::connections::DisconnectionReason; using ::nearby::analytics::PacketMetaData; -using DisconnectionReason = - ::location::nearby::proto::connections::DisconnectionReason; // We set this to 11s to provide sufficient time for an in-progress WebRTC // bandwidth upgrade to resolve. This is chosen to be slightly longer than the diff --git a/connections/implementation/fake_bwu_handler.h b/connections/implementation/fake_bwu_handler.h index 6f9aaa910f..d87e99f2d9 100644 --- a/connections/implementation/fake_bwu_handler.h +++ b/connections/implementation/fake_bwu_handler.h @@ -15,6 +15,7 @@ #ifndef NEARBY_CONNECTIONS_IMPLEMENTATION_FAKE_BWU_HANDLER_H_ #define NEARBY_CONNECTIONS_IMPLEMENTATION_FAKE_BWU_HANDLER_H_ +#include #include #include #include @@ -22,9 +23,15 @@ #include #include "connections/implementation/base_bwu_handler.h" +#include "connections/implementation/bwu_handler.h" #include "connections/implementation/bwu_manager.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" #include "connections/implementation/fake_endpoint_channel.h" +#include "connections/implementation/offline_frames.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/exception.h" +#include "internal/platform/expected.h" namespace nearby { namespace connections { @@ -104,14 +111,15 @@ class FakeBwuHandler : public BaseBwuHandler { }; // BwuHandler: - std::unique_ptr CreateUpgradedEndpointChannel( + ErrorOr> + CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) final { create_calls_.push_back({.client = client, .service_id = service_id, .endpoint_id = endpoint_id}); - return std::make_unique(medium_, service_id); + return {std::make_unique(medium_, service_id)}; } Medium GetUpgradeMedium() const final { return medium_; } diff --git a/connections/implementation/internal_payload_factory.cc b/connections/implementation/internal_payload_factory.cc index 38268fc9eb..382740c0be 100644 --- a/connections/implementation/internal_payload_factory.cc +++ b/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 CreateOutgoingInternalPayload( +ErrorOr> CreateOutgoingInternalPayload( Payload payload) { switch (payload.GetType()) { case PayloadType::kBytes: - return std::make_unique(std::move(payload)); + return {std::make_unique(std::move(payload))}; case PayloadType::kFile: { - return std::make_unique(std::move(payload)); + return { + std::make_unique(std::move(payload))}; } case PayloadType::kStream: - return std::make_unique( - std::move(payload)); + return { + std::make_unique(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 CreateIncomingInternalPayload( +ErrorOr> 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( - Payload(payload_id, ByteArray(frame.payload_chunk().body()))); + return {std::make_unique( + Payload(payload_id, ByteArray(frame.payload_chunk().body())))}; } case PayloadTransferFrame::PayloadHeader::STREAM: { auto [input, output] = CreatePipe(); - return std::make_unique( - Payload(payload_id, std::move(input)), std::move(output)); + return {std::make_unique( + Payload(payload_id, std::move(input)), std::move(output))}; } case PayloadTransferFrame::PayloadHeader::FILE: { @@ -397,7 +401,7 @@ std::unique_ptr 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 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( + return {std::make_unique( Payload(payload_id, InputFile(payload_id, total_size)), - OutputFile(payload_id), total_size); + OutputFile(payload_id), total_size)}; } else { - return std::make_unique( + return {std::make_unique( 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)}; } } diff --git a/connections/implementation/internal_payload_factory.h b/connections/implementation/internal_payload_factory.h index 6e510ade96..26a41b8015 100644 --- a/connections/implementation/internal_payload_factory.h +++ b/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 - #include +#include #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 CreateOutgoingInternalPayload(Payload payload); +ErrorOr> CreateOutgoingInternalPayload( + Payload payload); // Creates an InternalPayload representing an incoming Payload from a remote // endpoint. -std::unique_ptr CreateIncomingInternalPayload( +ErrorOr> CreateIncomingInternalPayload( const location::nearby::connections::PayloadTransferFrame& frame, const std::string& custom_save_path); diff --git a/connections/implementation/internal_payload_factory_test.cc b/connections/implementation/internal_payload_factory_test.cc index 9f6979aefc..d69e6d7bc4 100644 --- a/connections/implementation/internal_payload_factory_test.cc +++ b/connections/implementation/internal_payload_factory_test.cc @@ -14,6 +14,7 @@ #include "connections/implementation/internal_payload_factory.h" +#include #include #include #include @@ -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 internal_payload = + ErrorOr> result = CreateOutgoingInternalPayload(Payload{data}); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> result = CreateOutgoingInternalPayload(Payload(std::move(input))); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> result = CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)}); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> result = CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> result = CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> result = CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> 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 internal_payload = + ErrorOr> result = CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> result = CreateIncomingInternalPayload(frame, path); + ASSERT_FALSE(result.has_error()); + std::unique_ptr 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 internal_payload = + ErrorOr> interal_payload_result = CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)}); + ASSERT_FALSE(interal_payload_result.has_error()); + std::unique_ptr internal_payload = + std::move(interal_payload_result.value()); EXPECT_NE(internal_payload, nullptr); ExceptionOr 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 internal_payload = + ErrorOr> interal_payload_result = CreateOutgoingInternalPayload(Payload(std::move(input))); + ASSERT_FALSE(interal_payload_result.has_error()); + std::unique_ptr internal_payload = + std::move(interal_payload_result.value()); EXPECT_NE(internal_payload, nullptr); output->Write(contents); diff --git a/connections/implementation/payload_manager.cc b/connections/implementation/payload_manager.cc index 12d333e0e5..81abd3124b 100644 --- a/connections/implementation/payload_manager.cc +++ b/connections/implementation/payload_manager.cc @@ -23,26 +23,32 @@ #include #include +#include "absl/container/flat_hash_map.h" #include "absl/functional/any_invocable.h" #include "absl/functional/bind_front.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/time/clock.h" #include "absl/time/time.h" +#include "connections/implementation/analytics/packet_meta_data.h" #include "connections/implementation/analytics/throughput_recorder.h" #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel_manager.h" #include "connections/implementation/endpoint_manager.h" #include "connections/implementation/flags/nearby_connections_feature_flags.h" +#include "connections/implementation/internal_payload.h" #include "connections/implementation/internal_payload_factory.h" #include "connections/implementation/proto/offline_wire_formats.pb.h" #include "connections/listeners.h" +#include "connections/medium_selector.h" #include "connections/payload.h" #include "connections/payload_type.h" +#include "connections/status.h" #include "internal/flags/nearby_flags.h" #include "internal/platform/byte_array.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/exception.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" #include "internal/platform/logging.h" #include "internal/platform/mutex_lock.h" @@ -52,20 +58,19 @@ namespace nearby { namespace connections { +namespace { using ::location::nearby::connections::OfflineFrame; +using ::location::nearby::connections::PayloadTransferFrame; using ::location::nearby::connections::V1Frame; +using ::location::nearby::proto::connections::Medium; +using ::location::nearby::proto::connections::OperationResultCode; using ::location::nearby::proto::connections::PayloadStatus; -using ::nearby::analytics::PacketMetaData; +using PacketMetaData = ::nearby::analytics::PacketMetaData; using ::nearby::analytics::ThroughputRecorderContainer; -using ::nearby::connections::PayloadDirection; +using PayloadDirection = ::nearby::connections::PayloadDirection; -namespace { constexpr absl::Duration kMinTransferUpdateInterval = absl::Milliseconds(50); -} - -// C++14 requires to declare this. -// TODO(apolyudov): remove when migration to c++17 is possible. -constexpr absl::Duration PayloadManager::kWaitCloseTimeout; +} // namespace bool PayloadManager::SendPayloadLoop( ClientProxy* client, PendingPayload& pending_payload, @@ -82,6 +87,7 @@ bool PayloadManager::SendPayloadLoop( for (const auto& endpoint : unavailable_endpoints) { HandleFinishedOutgoingPayload( client, {endpoint->id}, payload_header, next_chunk_offset, + EndpointInfoStatusToOperationResultCode(endpoint->status.Get()), EndpointInfoStatusToPayloadStatus(endpoint->status.Get())); } @@ -100,10 +106,10 @@ bool PayloadManager::SendPayloadLoop( LOG(INFO) << "Aborting send of payload_id=" << pending_payload.GetInternalPayload()->GetId() << " at offset " << next_chunk_offset << " since it is marked canceled."; - HandleFinishedOutgoingPayload(client, available_endpoint_ids, - payload_header, next_chunk_offset, - location::nearby::proto::connections:: - PayloadStatus::LOCAL_CANCELLATION); + HandleFinishedOutgoingPayload( + client, available_endpoint_ids, payload_header, next_chunk_offset, + OperationResultCode::CLIENT_CANCELLATION_LOCAL_CANCEL_PAYLOAD, + PayloadStatus::LOCAL_CANCELLATION); return false; } @@ -119,9 +125,10 @@ bool PayloadManager::SendPayloadLoop( LOG(WARNING) << "PayloadManager failed to skip offset " << resume_offset << " on payload_id " << pending_payload.GetInternalPayload()->GetId(); - HandleFinishedOutgoingPayload( - client, available_endpoint_ids, payload_header, next_chunk_offset, - location::nearby::proto::connections::PayloadStatus::LOCAL_ERROR); + HandleFinishedOutgoingPayload(client, available_endpoint_ids, + payload_header, next_chunk_offset, + OperationResultCode::IO_FILE_READING_ERROR, + PayloadStatus::LOCAL_ERROR); return false; } NEARBY_VLOG(1) << "PayloadManager successfully skipped " @@ -151,7 +158,7 @@ bool PayloadManager::SendPayloadLoop( << pending_payload.GetInternalPayload()->GetId(); HandleFinishedOutgoingPayload( client, available_endpoint_ids, payload_header, next_chunk_offset, - location::nearby::proto::connections::PayloadStatus::LOCAL_ERROR); + OperationResultCode::IO_FILE_READING_ERROR, PayloadStatus::LOCAL_ERROR); return false; } @@ -168,10 +175,10 @@ bool PayloadManager::SendPayloadLoop( LOG(INFO) << "Payload xfer: endpoints failed: payload_id=" << payload_header.id() << "; endpoint_ids={" << ToString(failed_endpoint_ids) << "}", - HandleFinishedOutgoingPayload(client, failed_endpoint_ids, - payload_header, next_chunk_offset, - location::nearby::proto::connections:: - PayloadStatus::ENDPOINT_IO_ERROR); + HandleFinishedOutgoingPayload( + client, failed_endpoint_ids, payload_header, next_chunk_offset, + OperationResultCode::CONNECTIVITY_GENERIC_WRITING_CHANNEL_IO_ERROR, + PayloadStatus::ENDPOINT_IO_ERROR); } bool is_last_chunk = IsLastChunk(payload_chunk); // Check whether at least one endpoint succeeded -- if they all failed, @@ -298,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> 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 internal_payload = std::move(result.value()); Payload::Id payload_id = internal_payload->GetId(); LOG(INFO) << "CreateOutgoingPayload: payload_id=" << payload_id; MutexLock lock(&mutex_); @@ -413,9 +427,10 @@ void PayloadManager::SendPayload(ClientProxy* client, // with. This should never be reached since the ServiceControllerRouter has // already checked whether or not we can work with this Payload type. if (!executor) { - RecordInvalidPayloadAnalytics(client, endpoint_ids, payload.GetId(), - payload.GetType(), payload.GetOffset(), - payload_total_size); + RecordInvalidPayloadAnalytics( + client, endpoint_ids, payload.GetId(), payload.GetType(), + payload.GetOffset(), payload_total_size, + OperationResultCode::NEARBY_GENERIC_OUTGOING_PAYLOAD_CREATION_FAILURE); LOG(INFO) << "PayloadManager failed to determine the right executor for " "outgoing payload_id=" << payload.GetId() @@ -441,9 +456,11 @@ void PayloadManager::SendPayload(ClientProxy* client, if (shutdown_.Get()) return; PendingPayloadHandle pending_payload = GetPayload(payload_id); if (!pending_payload) { - RecordInvalidPayloadAnalytics(client, endpoint_ids, payload_id, - payload_type, resume_offset, - payload_total_size); + RecordInvalidPayloadAnalytics( + client, endpoint_ids, payload_id, payload_type, resume_offset, + payload_total_size, + OperationResultCode:: + NEARBY_GENERIC_OUTGOING_PAYLOAD_CREATION_FAILURE); LOG(INFO) << "PayloadManager failed to create InternalPayload for outgoing " "payload_id=" @@ -513,11 +530,11 @@ Status PayloadManager::CancelPayload(ClientProxy* client, } // @EndpointManagerDataPool -void PayloadManager::OnIncomingFrame( - OfflineFrame& offline_frame, const std::string& from_endpoint_id, - ClientProxy* to_client, - location::nearby::proto::connections::Medium current_medium, - PacketMetaData& packet_meta_data) { +void PayloadManager::OnIncomingFrame(OfflineFrame& offline_frame, + const std::string& from_endpoint_id, + ClientProxy* to_client, + Medium current_medium, + PacketMetaData& packet_meta_data) { PayloadTransferFrame& frame = *offline_frame.mutable_v1()->mutable_payload_transfer(); @@ -607,25 +624,36 @@ void PayloadManager::OnEndpointDisconnect(ClientProxy* client, client->OnPayloadProgress(endpoint_id, update); PayloadStatus payload_status; + OperationResultCode operation_result_code; switch (reason) { case DisconnectionReason::LOCAL_DISCONNECTION: payload_status = PayloadStatus::LOCAL_CLIENT_DISCONNECTION; + operation_result_code = + OperationResultCode::CLIENT_CANCELLATION_LOCAL_DISCONNECT; break; case DisconnectionReason::REMOTE_DISCONNECTION: payload_status = PayloadStatus::REMOTE_CLIENT_DISCONNECTION; + operation_result_code = + OperationResultCode::CLIENT_CANCELLATION_REMOTE_DISCONNECT; break; case DisconnectionReason::IO_ERROR: default: payload_status = PayloadStatus::ENDPOINT_IO_ERROR; + operation_result_code = + client->GetAnalyticsRecorder() + .GetChannelIoErrorResultCodeFromMedium( + client->GetConnectedMedium(endpoint_id)); break; } if (pending_payload->IsIncoming()) { client->GetAnalyticsRecorder().OnIncomingPayloadDone( - endpoint_id, pending_payload->GetId(), payload_status); + endpoint_id, pending_payload->GetId(), payload_status, + operation_result_code); } else { client->GetAnalyticsRecorder().OnOutgoingPayloadDone( - endpoint_id, pending_payload->GetId(), payload_status); + endpoint_id, pending_payload->GetId(), payload_status, + operation_result_code); } }); @@ -633,46 +661,69 @@ void PayloadManager::OnEndpointDisconnect(ClientProxy* client, }); } -location::nearby::proto::connections::PayloadStatus -PayloadManager::EndpointInfoStatusToPayloadStatus(EndpointInfo::Status status) { +PayloadStatus PayloadManager::EndpointInfoStatusToPayloadStatus( + EndpointInfo::Status status) { + switch (status) { + case EndpointInfo::Status::kCanceled: + return PayloadStatus::REMOTE_CANCELLATION; + case EndpointInfo::Status::kError: + return PayloadStatus::REMOTE_ERROR; + case EndpointInfo::Status::kAvailable: + return PayloadStatus::SUCCESS; + default: + LOG(INFO) << "PayloadManager: Unknown PayloadStatus"; + return PayloadStatus::UNKNOWN_PAYLOAD_STATUS; + } +} + +OperationResultCode PayloadManager::EndpointInfoStatusToOperationResultCode( + EndpointInfo::Status status) { switch (status) { case EndpointInfo::Status::kCanceled: - return location::nearby::proto::connections::PayloadStatus:: - REMOTE_CANCELLATION; + return OperationResultCode::CLIENT_CANCELLATION_REMOTE_IN_CANCELED_STATE; case EndpointInfo::Status::kError: - return location::nearby::proto::connections::PayloadStatus::REMOTE_ERROR; + return OperationResultCode::NEARBY_GENERIC_REMOTE_ENDPOINT_STATUS_ERROR; case EndpointInfo::Status::kAvailable: - return location::nearby::proto::connections::PayloadStatus::SUCCESS; + return OperationResultCode::DETAIL_SUCCESS; default: LOG(INFO) << "PayloadManager: Unknown PayloadStatus"; - return location::nearby::proto::connections::PayloadStatus:: - UNKNOWN_PAYLOAD_STATUS; + return OperationResultCode::DETAIL_UNKNOWN; } } -location::nearby::proto::connections::PayloadStatus -PayloadManager::ControlMessageEventToPayloadStatus( +PayloadStatus PayloadManager::ControlMessageEventToPayloadStatus( PayloadTransferFrame::ControlMessage::EventType event) { switch (event) { case PayloadTransferFrame::ControlMessage::PAYLOAD_ERROR: - return location::nearby::proto::connections::PayloadStatus::REMOTE_ERROR; + return PayloadStatus::REMOTE_ERROR; case PayloadTransferFrame::ControlMessage::PAYLOAD_CANCELED: - return location::nearby::proto::connections::PayloadStatus:: - REMOTE_CANCELLATION; + return PayloadStatus::REMOTE_CANCELLATION; default: LOG(INFO) << "PayloadManager: unknown event=" << event; - return location::nearby::proto::connections::PayloadStatus:: - UNKNOWN_PAYLOAD_STATUS; + return PayloadStatus::UNKNOWN_PAYLOAD_STATUS; + } +} + +OperationResultCode PayloadManager::ControlMessageEventToOperationResultCode( + PayloadTransferFrame::ControlMessage::EventType event) { + switch (event) { + case PayloadTransferFrame::ControlMessage::PAYLOAD_ERROR: + return OperationResultCode::NEARBY_GENERIC_REMOTE_REPORT_PAYLOADS_ERROR; + case PayloadTransferFrame::ControlMessage::PAYLOAD_CANCELED: + return OperationResultCode::CLIENT_CANCELLATION_REMOTE_CANCEL_PAYLOAD; + default: + LOG(INFO) << "PayloadManager: unknown event=" << event; + return OperationResultCode::DETAIL_UNKNOWN; } } PayloadProgressInfo::Status PayloadManager::PayloadStatusToTransferUpdateStatus( - location::nearby::proto::connections::PayloadStatus status) { + PayloadStatus status) { switch (status) { - case location::nearby::proto::connections::LOCAL_CANCELLATION: - case location::nearby::proto::connections::REMOTE_CANCELLATION: + case PayloadStatus::LOCAL_CANCELLATION: + case PayloadStatus::REMOTE_CANCELLATION: return PayloadProgressInfo::Status::kCanceled; - case location::nearby::proto::connections::SUCCESS: + case PayloadStatus::SUCCESS: return PayloadProgressInfo::Status::kSuccess; default: return PayloadProgressInfo::Status::kFailure; @@ -742,14 +793,15 @@ PayloadTransferFrame::PayloadChunk PayloadManager::CreatePayloadChunk( return payload_chunk; } -PayloadManager::PendingPayloadHandle PayloadManager::CreateIncomingPayload( - const PayloadTransferFrame& frame, const std::string& endpoint_id) { - auto internal_payload = +ErrorOr +PayloadManager::CreateIncomingPayload(const PayloadTransferFrame& frame, + const std::string& endpoint_id) { + ErrorOr> result = CreateIncomingInternalPayload(frame, custom_save_path_); - if (!internal_payload) { - return PendingPayloadHandle(); + if (result.has_error()) { + return {result.error()}; } - + std::unique_ptr internal_payload = std::move(result.value()); Payload::Id payload_id = internal_payload->GetId(); LOG(INFO) << "CreateIncomingPayload: payload_id=" << payload_id; pending_payloads_.StartTrackingPayload( @@ -757,7 +809,7 @@ PayloadManager::PendingPayloadHandle PayloadManager::CreateIncomingPayload( std::make_unique( std::move(internal_payload), EndpointIds{endpoint_id}, true, absl::bind_front(&PayloadManager::OnPendingPayloadDestroy, this))); - return pending_payloads_.GetPayload(payload_id); + return {pending_payloads_.GetPayload(payload_id)}; } void PayloadManager::OnPendingPayloadDestroy(const PendingPayload* payload) { @@ -776,13 +828,13 @@ void PayloadManager::OnPendingPayloadDestroy(const PendingPayload* payload) { void PayloadManager::SendClientCallbacksForFinishedOutgoingPayload( ClientProxy* client, const EndpointIds& finished_endpoint_ids, const PayloadTransferFrame::PayloadHeader& payload_header, - std::int64_t num_bytes_successfully_transferred, - location::nearby::proto::connections::PayloadStatus status) { + std::int64_t num_bytes_successfully_transferred, PayloadStatus status, + OperationResultCode operation_result_code) { RunOnStatusUpdateThread( "outgoing-payload-callbacks", [this, client, finished_endpoint_ids, payload_header, - num_bytes_successfully_transferred, - status]() RUN_ON_PAYLOAD_STATUS_UPDATE_THREAD() { + num_bytes_successfully_transferred, status, + operation_result_code]() RUN_ON_PAYLOAD_STATUS_UPDATE_THREAD() { // Make sure we're still tracking this payload. PendingPayloadHandle pending_payload = GetPayload(payload_header.id()); if (!pending_payload) { @@ -805,7 +857,13 @@ void PayloadManager::SendClientCallbacksForFinishedOutgoingPayload( // Mark this payload as done for analytics. client->GetAnalyticsRecorder().OnOutgoingPayloadDone( - endpoint_id, payload_header.id(), status); + endpoint_id, payload_header.id(), status, + (operation_result_code == OperationResultCode::DETAIL_UNKNOWN && + status == PayloadStatus::ENDPOINT_IO_ERROR) + ? client->GetAnalyticsRecorder() + .GetChannelIoErrorResultCodeFromMedium( + client->GetConnectedMedium(endpoint_id)) + : operation_result_code); } // Remove these endpoints from our tracking list for this payload. @@ -821,12 +879,12 @@ void PayloadManager::SendClientCallbacksForFinishedOutgoingPayload( void PayloadManager::SendClientCallbacksForFinishedIncomingPayload( ClientProxy* client, const std::string& endpoint_id, const PayloadTransferFrame::PayloadHeader& payload_header, - std::int64_t offset_bytes, - location::nearby::proto::connections::PayloadStatus status) { + std::int64_t offset_bytes, PayloadStatus status, + OperationResultCode operation_result_code) { RunOnStatusUpdateThread( "incoming-payload-callbacks", - [this, client, endpoint_id, payload_header, offset_bytes, - status]() RUN_ON_PAYLOAD_STATUS_UPDATE_THREAD() { + [this, client, endpoint_id, payload_header, offset_bytes, status, + operation_result_code]() RUN_ON_PAYLOAD_STATUS_UPDATE_THREAD() { // Make sure we're still tracking this payload. PendingPayloadHandle pending_payload = GetPayload(payload_header.id()); if (!pending_payload) { @@ -845,7 +903,7 @@ void PayloadManager::SendClientCallbacksForFinishedIncomingPayload( // Analyze client->GetAnalyticsRecorder().OnIncomingPayloadDone( - endpoint_id, payload_header.id(), status); + endpoint_id, payload_header.id(), status, operation_result_code); }); } @@ -918,10 +976,10 @@ bool PayloadManager::WaitForReceivedAck( // Local payload cancellation if (latest_pending_payload->IsLocallyCanceled()) { - HandleFinishedOutgoingPayload(client, {endpoint_id}, payload_header, - payload_chunk_offset, - location::nearby::proto::connections:: - PayloadStatus::LOCAL_CANCELLATION); + HandleFinishedOutgoingPayload( + client, {endpoint_id}, payload_header, payload_chunk_offset, + OperationResultCode::CLIENT_CANCELLATION_LOCAL_CANCEL_PAYLOAD, + PayloadStatus::LOCAL_CANCELLATION); LOG(INFO) << "[safe-to-disconnect] short-circuiting local " "payload cancellation for " << payload_header.id() << ", stop wait ack."; @@ -932,6 +990,7 @@ bool PayloadManager::WaitForReceivedAck( endpoint_info->status.Get())) { HandleFinishedOutgoingPayload( client, {endpoint_id}, payload_header, payload_chunk_offset, + OperationResultCode::CLIENT_CANCELLATION_REMOTE_CANCEL_PAYLOAD, EndpointInfoStatusToPayloadStatus(endpoint_info->status.Get())); LOG(INFO) << "[safe-to-disconnect] short-circuiting remote " "payload cancellation for " @@ -993,20 +1052,19 @@ void PayloadManager::HandleFinishedOutgoingPayload( ClientProxy* client, const EndpointIds& finished_endpoint_ids, const PayloadTransferFrame::PayloadHeader& payload_header, std::int64_t num_bytes_successfully_transferred, - location::nearby::proto::connections::PayloadStatus status) { + OperationResultCode operation_result_code, PayloadStatus status) { // This call will destroy a pending payload. SendClientCallbacksForFinishedOutgoingPayload( client, finished_endpoint_ids, payload_header, - num_bytes_successfully_transferred, status); + num_bytes_successfully_transferred, status, operation_result_code); switch (status) { - case location::nearby::proto::connections::PayloadStatus::LOCAL_ERROR: + case PayloadStatus::LOCAL_ERROR: SendControlMessage(finished_endpoint_ids, payload_header, num_bytes_successfully_transferred, PayloadTransferFrame::ControlMessage::PAYLOAD_ERROR); break; - case location::nearby::proto::connections::PayloadStatus:: - LOCAL_CANCELLATION: + case PayloadStatus::LOCAL_CANCELLATION: LOG(INFO) << "Sending PAYLOAD_CANCEL to receiver side; payload_id=" << payload_header.id(); SendControlMessage( @@ -1014,7 +1072,7 @@ void PayloadManager::HandleFinishedOutgoingPayload( num_bytes_successfully_transferred, PayloadTransferFrame::ControlMessage::PAYLOAD_CANCELED); break; - case location::nearby::proto::connections::PayloadStatus::ENDPOINT_IO_ERROR: + case PayloadStatus::ENDPOINT_IO_ERROR: // Unregister these endpoints, since we had an IO error on the physical // connection. for (const auto& endpoint_id : finished_endpoint_ids) { @@ -1022,9 +1080,8 @@ void PayloadManager::HandleFinishedOutgoingPayload( DisconnectionReason::IO_ERROR); } break; - case location::nearby::proto::connections::PayloadStatus::REMOTE_ERROR: - case location::nearby::proto::connections::PayloadStatus:: - REMOTE_CANCELLATION: + case PayloadStatus::REMOTE_ERROR: + case PayloadStatus::REMOTE_CANCELLATION: // No special handling needed for these. break; default: @@ -1038,18 +1095,18 @@ void PayloadManager::HandleFinishedOutgoingPayload( void PayloadManager::HandleFinishedIncomingPayload( ClientProxy* client, const std::string& endpoint_id, const PayloadTransferFrame::PayloadHeader& payload_header, - std::int64_t offset_bytes, - location::nearby::proto::connections::PayloadStatus status) { - SendClientCallbacksForFinishedIncomingPayload( - client, endpoint_id, payload_header, offset_bytes, status); + std::int64_t offset_bytes, PayloadStatus status, + OperationResultCode operation_result_code) { + SendClientCallbacksForFinishedIncomingPayload(client, endpoint_id, + payload_header, offset_bytes, + status, operation_result_code); switch (status) { - case location::nearby::proto::connections::PayloadStatus::LOCAL_ERROR: + case PayloadStatus::LOCAL_ERROR: SendControlMessage({endpoint_id}, payload_header, offset_bytes, PayloadTransferFrame::ControlMessage::PAYLOAD_ERROR); break; - case location::nearby::proto::connections::PayloadStatus:: - LOCAL_CANCELLATION: + case PayloadStatus::LOCAL_CANCELLATION: SendControlMessage( {endpoint_id}, payload_header, offset_bytes, PayloadTransferFrame::ControlMessage::PAYLOAD_CANCELED); @@ -1138,8 +1195,8 @@ void PayloadManager::HandleSuccessfulOutgoingChunk( if (is_last_chunk) { client->GetAnalyticsRecorder().OnOutgoingPayloadDone( - endpoint_id, payload_header.id(), - location::nearby::proto::connections::SUCCESS); + endpoint_id, payload_header.id(), PayloadStatus::SUCCESS, + OperationResultCode::DETAIL_SUCCESS); // Stop tracking this endpoint. pending_payload->RemoveEndpoints({endpoint_id}); @@ -1235,8 +1292,8 @@ void PayloadManager::HandleSuccessfulIncomingChunk( if (is_last_chunk) { DestroyPendingPayload(payload_header.id()); client->GetAnalyticsRecorder().OnIncomingPayloadDone( - endpoint_id, payload_header.id(), - location::nearby::proto::connections::SUCCESS); + endpoint_id, payload_header.id(), PayloadStatus::SUCCESS, + OperationResultCode::DETAIL_SUCCESS); } else { client->GetAnalyticsRecorder().OnPayloadChunkReceived( endpoint_id, payload_header.id(), payload_chunk_body_size); @@ -1286,18 +1343,33 @@ void PayloadManager::ProcessDataPacket( payload_header.total_size()); }); - pending_payload = + ErrorOr result = CreateIncomingPayload(payload_transfer_frame, from_endpoint_id); - if (!pending_payload) { + if (result.has_error()) { LOG(WARNING) << "PayloadManager failed to create InternalPayload from " "PayloadTransferFrame with payload_id=" << payload_header.id() << " and type " << payload_header.type() << ", aborting receipt."; + + // Analyticize. + OperationResultCode operation_result_code = + result.error().operation_result_code().value(); + RunOnStatusUpdateThread( + "process-data-packet", + [to_client, from_endpoint_id, payload_header, operation_result_code]() + RUN_ON_PAYLOAD_STATUS_UPDATE_THREAD() { + to_client->GetAnalyticsRecorder().OnIncomingPayloadDone( + from_endpoint_id, payload_header.id(), + PayloadStatus::LOCAL_ERROR, operation_result_code); + }); + // Send the error to the remote endpoint. SendControlMessage({from_endpoint_id}, payload_header, payload_chunk.offset(), PayloadTransferFrame::ControlMessage::PAYLOAD_ERROR); return; + } else { + pending_payload = std::move(result.value()); } // Also, let the client know of this new incoming payload. RunOnStatusUpdateThread( @@ -1327,10 +1399,10 @@ void PayloadManager::ProcessDataPacket( // do all the cleanup. See go/nc-cancel-payload LOG(INFO) << "ProcessDataPacket: [cancel] endpoint_id=" << from_endpoint_id << "; payload_id=" << pending_payload->GetId(); - HandleFinishedIncomingPayload(to_client, from_endpoint_id, payload_header, - payload_chunk.offset(), - location::nearby::proto::connections:: - PayloadStatus::LOCAL_CANCELLATION); + HandleFinishedIncomingPayload( + to_client, from_endpoint_id, payload_header, payload_chunk.offset(), + PayloadStatus::LOCAL_CANCELLATION, + OperationResultCode::CLIENT_CANCELLATION_LOCAL_CANCEL_PAYLOAD); return; } @@ -1354,7 +1426,7 @@ void PayloadManager::ProcessDataPacket( << "; payload_id=" << pending_payload->GetId(); HandleFinishedIncomingPayload( to_client, from_endpoint_id, payload_header, payload_chunk.offset(), - location::nearby::proto::connections::PayloadStatus::LOCAL_ERROR); + PayloadStatus::LOCAL_ERROR, OperationResultCode::IO_FILE_WRITING_ERROR); return; } packet_meta_data.StopFileIo(); @@ -1404,7 +1476,8 @@ void PayloadManager::ProcessControlPacket( HandleFinishedIncomingPayload( to_client, from_endpoint_id, payload_header, control_message.offset(), - ControlMessageEventToPayloadStatus(control_message.event())); + ControlMessageEventToPayloadStatus(control_message.event()), + ControlMessageEventToOperationResultCode(control_message.event())); } else { LOG(INFO) << "Outgoing PAYLOAD_CANCELED: from endpoint_id=" << from_endpoint_id << "; self=" << this; @@ -1423,7 +1496,8 @@ void PayloadManager::ProcessControlPacket( HandleFinishedIncomingPayload( to_client, from_endpoint_id, payload_header, control_message.offset(), - ControlMessageEventToPayloadStatus(control_message.event())); + ControlMessageEventToPayloadStatus(control_message.event()), + ControlMessageEventToOperationResultCode(control_message.event())); } else { pending_payload->SetEndpointStatusFromControlMessage(from_endpoint_id, control_message); @@ -1480,14 +1554,14 @@ void PayloadManager::RecordPayloadStartedAnalytics( void PayloadManager::RecordInvalidPayloadAnalytics( ClientProxy* client, const EndpointIds& endpoint_ids, std::int64_t payload_id, PayloadType payload_type, std::int64_t offset, - std::int64_t total_size) { + std::int64_t total_size, OperationResultCode operation_result_code) { RecordPayloadStartedAnalytics(client, endpoint_ids, payload_id, payload_type, offset, total_size); for (const auto& endpoint_id : endpoint_ids) { client->GetAnalyticsRecorder().OnOutgoingPayloadDone( - endpoint_id, payload_id, - location::nearby::proto::connections::LOCAL_ERROR); + endpoint_id, payload_id, PayloadStatus::LOCAL_ERROR, + operation_result_code); } } @@ -1665,8 +1739,7 @@ void PayloadManager::RunOnStatusUpdateThread( payload_status_update_executor_.Execute(name, std::move(runnable)); } -/////////////////////////////// PendingPayloads -////////////////////////////////// +/////////////////////////////// PendingPayloads //////////////////////////////// void PayloadManager::PendingPayloads::StartTrackingPayload( Payload::Id payload_id, std::unique_ptr pending_payload) { diff --git a/connections/implementation/payload_manager.h b/connections/implementation/payload_manager.h index a0da66d10b..3f4891788a 100644 --- a/connections/implementation/payload_manager.h +++ b/connections/implementation/payload_manager.h @@ -17,7 +17,6 @@ #include #include -#include #include #include #include @@ -40,12 +39,12 @@ #include "internal/platform/byte_array.h" #include "internal/platform/condition_variable.h" #include "internal/platform/count_down_latch.h" +#include "internal/platform/expected.h" #include "internal/platform/mutex.h" #include "internal/platform/single_thread_executor.h" namespace nearby { namespace connections { -using ::location::nearby::connections::PayloadTransferFrame; // Annotations for methods that need to run on PayloadStatusUpdateThread. // Use only in PayloadManager @@ -55,8 +54,7 @@ using ::location::nearby::connections::PayloadTransferFrame; class PayloadManager : public EndpointManager::FrameProcessor { public: using EndpointIds = std::vector; - constexpr static const absl::Duration kWaitCloseTimeout = - absl::Milliseconds(5000); + static constexpr absl::Duration kWaitCloseTimeout = absl::Milliseconds(5000); explicit PayloadManager(EndpointManager& endpoint_manager); ~PayloadManager() override; @@ -73,10 +71,11 @@ class PayloadManager : public EndpointManager::FrameProcessor { analytics::PacketMetaData& packet_meta_data) override; // @EndpointManagerThread - void OnEndpointDisconnect(ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - CountDownLatch barrier, - DisconnectionReason reason) override; + void OnEndpointDisconnect( + ClientProxy* client, const std::string& service_id, + const std::string& endpoint_id, CountDownLatch barrier, + location::nearby::proto::connections::DisconnectionReason reason) + override; void DisconnectFromEndpointManager(); @@ -94,10 +93,12 @@ class PayloadManager : public EndpointManager::FrameProcessor { }; void SetStatusFromControlMessage( - const PayloadTransferFrame::ControlMessage& control_message); + const location::nearby::connections::PayloadTransferFrame:: + ControlMessage& control_message); static Status ControlMessageEventToEndpointInfoStatus( - PayloadTransferFrame::ControlMessage::EventType event); + location::nearby::connections::PayloadTransferFrame::ControlMessage:: + EventType event); void MarkReceivedAckFromEndpoint(); bool IsEndpointAvailable(ClientProxy* clientProxy, EndpointInfo::Status status); @@ -153,8 +154,8 @@ class PayloadManager : public EndpointManager::FrameProcessor { // Sets the status for a particular endpoint. void SetEndpointStatusFromControlMessage( const std::string& endpoint_id, - const PayloadTransferFrame::ControlMessage& control_message) - ABSL_LOCKS_EXCLUDED(mutex_); + const location::nearby::connections::PayloadTransferFrame:: + ControlMessage& control_message) ABSL_LOCKS_EXCLUDED(mutex_); // Sets the offset for a particular endpoint. void SetOffsetForEndpoint(const std::string& endpoint_id, @@ -272,47 +273,61 @@ class PayloadManager : public EndpointManager::FrameProcessor { // Returns list of endpoint ids. static EndpointIds EndpointsToEndpointIds(const Endpoints& endpoints); - bool SendPayloadLoop(ClientProxy* client, PendingPayload& pending_payload, - PayloadTransferFrame::PayloadHeader& payload_header, - std::int64_t& next_chunk_offset, size_t resume_offset, - int index); + bool SendPayloadLoop( + ClientProxy* client, PendingPayload& pending_payload, + location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, + std::int64_t& next_chunk_offset, size_t resume_offset, int index); void SendClientCallbacksForFinishedIncomingPayloadRunnable( ClientProxy* client, const std::string& endpoint_id, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t offset_bytes, - location::nearby::proto::connections::PayloadStatus status); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code); // Converts the status of an endpoint that's been set out-of-band via a // remote ControlMessage to the PayloadStatus for handling of that // endpoint-payload pair. static location::nearby::proto::connections::PayloadStatus EndpointInfoStatusToPayloadStatus(EndpointInfo::Status status); + static location::nearby::proto::connections::OperationResultCode + EndpointInfoStatusToOperationResultCode(EndpointInfo::Status status); // Converts a ControlMessage::EventType for a particular payload to a // PayloadStatus. Called when we've received a ControlMessage with this // event from a remote endpoint; thus the PayloadStatuses are REMOTE_*. static location::nearby::proto::connections::PayloadStatus ControlMessageEventToPayloadStatus( - PayloadTransferFrame::ControlMessage::EventType event); + location::nearby::connections::PayloadTransferFrame::ControlMessage:: + EventType event); + static location::nearby::proto::connections::OperationResultCode + ControlMessageEventToOperationResultCode( + location::nearby::connections::PayloadTransferFrame::ControlMessage:: + EventType event); static PayloadProgressInfo::Status PayloadStatusToTransferUpdateStatus( location::nearby::proto::connections::PayloadStatus status); int GetOptimalChunkSize(EndpointIds endpoint_ids); - PayloadTransferFrame::PayloadHeader CreatePayloadHeader( - const InternalPayload& internal_payload, size_t offset, - const std::string& parent_folder, const std::string& file_name); + location::nearby::connections::PayloadTransferFrame::PayloadHeader + CreatePayloadHeader(const InternalPayload& internal_payload, size_t offset, + const std::string& parent_folder, + const std::string& file_name); - PayloadTransferFrame::PayloadChunk CreatePayloadChunk(std::int64_t offset, - ByteArray body, - int index); - bool IsLastChunk(PayloadTransferFrame::PayloadChunk payload_chunk) { + location::nearby::connections::PayloadTransferFrame::PayloadChunk + CreatePayloadChunk(std::int64_t offset, ByteArray body, int index); + bool IsLastChunk( + location::nearby::connections::PayloadTransferFrame::PayloadChunk + payload_chunk) { return ((payload_chunk.flags() & - PayloadTransferFrame::PayloadChunk::LAST_CHUNK) != 0); + location::nearby::connections::PayloadTransferFrame::PayloadChunk:: + LAST_CHUNK) != 0); } - PendingPayloadHandle CreateIncomingPayload(const PayloadTransferFrame& frame, - const std::string& endpoint_id) - ABSL_LOCKS_EXCLUDED(mutex_); + ErrorOr CreateIncomingPayload( + const location::nearby::connections::PayloadTransferFrame& frame, + const std::string& endpoint_id) ABSL_LOCKS_EXCLUDED(mutex_); Payload::Id CreateOutgoingPayload(Payload payload, const EndpointIds& endpoint_ids) @@ -320,20 +335,28 @@ class PayloadManager : public EndpointManager::FrameProcessor { void SendClientCallbacksForFinishedOutgoingPayload( ClientProxy* client, const EndpointIds& finished_endpoint_ids, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t num_bytes_successfully_transferred, - location::nearby::proto::connections::PayloadStatus status); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code); void SendClientCallbacksForFinishedIncomingPayload( ClientProxy* client, const std::string& endpoint_id, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t offset_bytes, - location::nearby::proto::connections::PayloadStatus status); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code); void SendControlMessage( const EndpointIds& endpoint_ids, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t num_bytes_successfully_transferred, - PayloadTransferFrame::ControlMessage::EventType event_type); + location::nearby::connections::PayloadTransferFrame::ControlMessage:: + EventType event_type); void SendPayloadReceivedAck(ClientProxy* client, PendingPayload& pending_payload, @@ -343,7 +366,8 @@ class PayloadManager : public EndpointManager::FrameProcessor { bool WaitForReceivedAck( ClientProxy* client, const std::string& endpoint_id, PendingPayload& pending_payload, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t payload_chunk_offset, bool is_last_chunk); bool IsPayloadReceivedAckEnabled(ClientProxy* client, const std::string& endpoint_id, @@ -353,37 +377,49 @@ class PayloadManager : public EndpointManager::FrameProcessor { // statuses except for SUCCESS are handled here. void HandleFinishedOutgoingPayload( ClientProxy* client, const EndpointIds& finished_endpoint_ids, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t num_bytes_successfully_transferred, + location::nearby::proto::connections::OperationResultCode + operation_result_code, location::nearby::proto::connections::PayloadStatus status = location:: nearby::proto::connections::PayloadStatus::UNKNOWN_PAYLOAD_STATUS); void HandleFinishedIncomingPayload( ClientProxy* client, const std::string& endpoint_id, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int64_t offset_bytes, - location::nearby::proto::connections::PayloadStatus status); + location::nearby::proto::connections::PayloadStatus status, + location::nearby::proto::connections::OperationResultCode + operation_result_code); void HandleSuccessfulOutgoingChunk( ClientProxy* client, const std::string& endpoint_id, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int32_t payload_chunk_flags, std::int64_t payload_chunk_offset, std::int64_t payload_chunk_body_size); void HandleSuccessfulIncomingChunk( ClientProxy* client, const std::string& endpoint_id, - const PayloadTransferFrame::PayloadHeader& payload_header, + const location::nearby::connections::PayloadTransferFrame::PayloadHeader& + payload_header, std::int32_t payload_chunk_flags, std::int64_t payload_chunk_offset, std::int64_t payload_chunk_body_size); void ProcessDataPacket(ClientProxy* to_client, const std::string& from_endpoint_id, - PayloadTransferFrame& payload_transfer_frame, - Medium medium, + location::nearby::connections::PayloadTransferFrame& + payload_transfer_frame, + location::nearby::proto::connections::Medium medium, analytics::PacketMetaData& packet_meta_data); void ProcessControlPacket(ClientProxy* to_client, const std::string& from_endpoint_id, - PayloadTransferFrame& payload_transfer_frame); - void ProcessPayloadAckPacket(const std::string& from_endpoint_id, - PayloadTransferFrame& payload_transfer_frame); + location::nearby::connections::PayloadTransferFrame& + payload_transfer_frame); + void ProcessPayloadAckPacket( + const std::string& from_endpoint_id, + location::nearby::connections::PayloadTransferFrame& + payload_transfer_frame); void NotifyClientOfIncomingPayloadProgressInfo( ClientProxy* client, const std::string& endpoint_id, @@ -407,15 +443,16 @@ class PayloadManager : public EndpointManager::FrameProcessor { PayloadType payload_type, std::int64_t offset, std::int64_t total_size); - void RecordInvalidPayloadAnalytics(ClientProxy* client, - const EndpointIds& endpoint_ids, - std::int64_t payload_id, - PayloadType payload_type, - std::int64_t offset, - std::int64_t total_size); + void RecordInvalidPayloadAnalytics( + ClientProxy* client, const EndpointIds& endpoint_ids, + std::int64_t payload_id, PayloadType payload_type, std::int64_t offset, + std::int64_t total_size, + location::nearby::proto::connections::OperationResultCode + operation_result_code); PayloadType FramePayloadTypeToPayloadType( - PayloadTransferFrame::PayloadHeader::PayloadType type); + location::nearby::connections::PayloadTransferFrame::PayloadHeader:: + PayloadType type); void OnPendingPayloadDestroy(const PendingPayload* payload); mutable Mutex mutex_; diff --git a/connections/implementation/payload_manager_test.cc b/connections/implementation/payload_manager_test.cc index 797b0b162b..22d759f4d2 100644 --- a/connections/implementation/payload_manager_test.cc +++ b/connections/implementation/payload_manager_test.cc @@ -22,17 +22,16 @@ #include "absl/strings/string_view.h" #include "absl/time/time.h" #include "connections/implementation/analytics/packet_meta_data.h" -#include "connections/implementation/flags/nearby_connections_feature_flags.h" #include "connections/implementation/offline_frames.h" #include "connections/implementation/simulation_user.h" #include "connections/listeners.h" #include "connections/medium_selector.h" #include "connections/payload.h" #include "connections/status.h" -#include "internal/flags/nearby_flags.h" #include "internal/platform/byte_array.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/exception.h" +#include "internal/platform/input_stream.h" #include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" #include "internal/platform/pipe.h" @@ -41,6 +40,7 @@ namespace nearby { namespace connections { namespace { using ::location::nearby::connections::OfflineFrame; +using ::location::nearby::connections::PayloadTransferFrame; using ::nearby::analytics::PacketMetaData; using ::location::nearby::proto::connections::Medium; diff --git a/connections/implementation/webrtc_bwu_handler.cc b/connections/implementation/webrtc_bwu_handler.cc index 8d2b0652bc..3b470f23ac 100644 --- a/connections/implementation/webrtc_bwu_handler.cc +++ b/connections/implementation/webrtc_bwu_handler.cc @@ -32,13 +32,19 @@ #include "connections/implementation/offline_frames.h" #include "connections/implementation/webrtc_endpoint_channel.h" #include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" #include "internal/platform/logging.h" namespace nearby { namespace connections { + +namespace { using ::location::nearby::connections::LocationHint; using ::location::nearby::connections::LocationStandard; +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace +// TODO(edwinwu): Add exact OperationResultCode for WebrtcBwuHandler. WebrtcBwuHandler::WebrtcIncomingSocket::WebrtcIncomingSocket( const std::string& name, mediums::WebRtcSocketWrapper socket) : name_(name), socket_(socket) {} @@ -54,7 +60,7 @@ WebrtcBwuHandler::WebrtcBwuHandler( // Called by BWU target. Retrieves a new medium info from incoming message, // and establishes connection over WebRTC using this info. -std::unique_ptr +ErrorOr> WebrtcBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { @@ -79,7 +85,7 @@ WebrtcBwuHandler::CreateUpgradedEndpointChannel( NEARBY_LOGS(ERROR) << "WebRtcBwuHandler failed to connect to remote peer (" << peer_id.GetId() << ") on endpoint " << endpoint_id << ", aborting upgrade."; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } NEARBY_LOGS(INFO) << "WebRtcBwuHandler successfully connected to remote " @@ -95,9 +101,10 @@ WebrtcBwuHandler::CreateUpgradedEndpointChannel( NEARBY_LOGS(ERROR) << "WebRtcBwuHandler failed to create new EndpointChannel for " "outgoing socket, aborting upgrade."; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } - return channel; + return {std::move(channel)}; } void WebrtcBwuHandler::HandleRevertInitiatorStateForService( diff --git a/connections/implementation/webrtc_bwu_handler.h b/connections/implementation/webrtc_bwu_handler.h index 97e6cf4408..8ad821b88d 100644 --- a/connections/implementation/webrtc_bwu_handler.h +++ b/connections/implementation/webrtc_bwu_handler.h @@ -19,6 +19,7 @@ #include #include +#include #include "connections/implementation/base_bwu_handler.h" #include "connections/implementation/bwu_handler.h" @@ -29,6 +30,7 @@ #include "connections/implementation/mediums/webrtc_socket.h" #include "connections/medium_selector.h" #include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" namespace nearby { namespace connections { @@ -56,11 +58,14 @@ class WebrtcBwuHandler : public BaseBwuHandler { }; // BwuHandler implementation: - std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) final; - Medium GetUpgradeMedium() const final { return Medium::WEB_RTC; } + ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) final; + location::nearby::proto::connections::Medium GetUpgradeMedium() const final { + return Medium::WEB_RTC; + } void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) final {} diff --git a/connections/implementation/webrtc_bwu_handler_stub.cc b/connections/implementation/webrtc_bwu_handler_stub.cc index 91f19c1fd4..4bbb292f4b 100644 --- a/connections/implementation/webrtc_bwu_handler_stub.cc +++ b/connections/implementation/webrtc_bwu_handler_stub.cc @@ -25,10 +25,15 @@ #include "connections/implementation/mediums/webrtc_peer_id_stub.h" #include "connections/implementation/offline_frames.h" #include "connections/implementation/webrtc_endpoint_channel.h" +#include "internal/platform/expected.h" namespace nearby { namespace connections { +namespace { +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace + WebrtcBwuHandler::WebrtcIncomingSocket::WebrtcIncomingSocket( const std::string& name, mediums::WebRtcSocketWrapper socket) : name_(name), socket_(socket) {} @@ -44,11 +49,11 @@ WebrtcBwuHandler::WebrtcBwuHandler( // Called by BWU target. Retrieves a new medium info from incoming message, // and establishes connection over WebRTC using this info. -std::unique_ptr +ErrorOr> WebrtcBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } void WebrtcBwuHandler::HandleRevertInitiatorStateForService( diff --git a/connections/implementation/webrtc_bwu_handler_stub.h b/connections/implementation/webrtc_bwu_handler_stub.h index de0a9ff1fe..e3b0aebbbb 100644 --- a/connections/implementation/webrtc_bwu_handler_stub.h +++ b/connections/implementation/webrtc_bwu_handler_stub.h @@ -28,6 +28,7 @@ #else #include "connections/implementation/mediums/webrtc_socket.h" #endif +#include "internal/platform/expected.h" namespace nearby { namespace connections { @@ -55,11 +56,14 @@ class WebrtcBwuHandler : public BaseBwuHandler { }; // BwuHandler implementation: - std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) final; - Medium GetUpgradeMedium() const final { return Medium::WEB_RTC; } + ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) final; + location::nearby::proto::connections::Medium GetUpgradeMedium() const final { + return Medium::WEB_RTC; + } void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) final {} diff --git a/connections/implementation/wifi_direct_bwu_handler.cc b/connections/implementation/wifi_direct_bwu_handler.cc index 742aa002b7..e7e11157a1 100644 --- a/connections/implementation/wifi_direct_bwu_handler.cc +++ b/connections/implementation/wifi_direct_bwu_handler.cc @@ -15,7 +15,6 @@ #include "connections/implementation/wifi_direct_bwu_handler.h" #include -#include #include #include #include @@ -28,6 +27,7 @@ #include "connections/implementation/offline_frames.h" #include "connections/implementation/wifi_direct_endpoint_channel.h" #include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" #include "internal/platform/logging.h" #include "internal/platform/wifi_credential.h" #include "internal/platform/wifi_direct.h" @@ -35,6 +35,11 @@ namespace nearby { namespace connections { +namespace { +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace + +// TODO(edwinwu): Add exact OperationResultCode for WifiDirectBwuHandler. WifiDirectBwuHandler::WifiDirectBwuHandler( Mediums& mediums, IncomingConnectionCallback incoming_connection_callback) : BaseBwuHandler(std::move(incoming_connection_callback)), @@ -101,13 +106,13 @@ void WifiDirectBwuHandler::HandleRevertInitiatorStateForService( << "upgrade service ID " << upgrade_service_id; } -std::unique_ptr +ErrorOr> WifiDirectBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { if (!upgrade_path_info.has_wifi_direct_credentials()) { NEARBY_LOGS(INFO) << "No WifiDirect Credential"; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } const UpgradePathInfo::WifiDirectCredentials& upgrade_path_info_credentials = upgrade_path_info.wifi_direct_credentials(); @@ -123,7 +128,7 @@ WifiDirectBwuHandler::CreateUpgradedEndpointChannel( if (!wifi_direct_medium_.ConnectWifiDirect(ssid, password)) { NEARBY_LOGS(ERROR) << "Connect to WifiDiret GO failed"; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } WifiDirectSocket socket = wifi_direct_medium_.Connect( @@ -132,7 +137,7 @@ WifiDirectBwuHandler::CreateUpgradedEndpointChannel( NEARBY_LOGS(ERROR) << "WifiDirectBwuHandler failed to connect to the WifiDirect service(" << port << ") for endpoint " << endpoint_id; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } NEARBY_VLOG(1) @@ -140,8 +145,8 @@ WifiDirectBwuHandler::CreateUpgradedEndpointChannel( << port << ") while upgrading endpoint " << endpoint_id; // Create a new WifiDirectEndpointChannel. - return std::make_unique( - service_id, /*channel_name=*/service_id, socket); + return {std::make_unique( + service_id, /*channel_name=*/service_id, socket)}; } void WifiDirectBwuHandler::OnIncomingWifiDirectConnection( diff --git a/connections/implementation/wifi_direct_bwu_handler.h b/connections/implementation/wifi_direct_bwu_handler.h index e7f159ea23..c189b87d5f 100644 --- a/connections/implementation/wifi_direct_bwu_handler.h +++ b/connections/implementation/wifi_direct_bwu_handler.h @@ -19,8 +19,16 @@ #include #include "connections/implementation/base_bwu_handler.h" +#include "connections/implementation/bwu_handler.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" #include "connections/implementation/mediums/mediums.h" +#include "connections/implementation/mediums/wifi.h" +#include "connections/implementation/mediums/wifi_direct.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" +#include "internal/platform/wifi_direct.h" +#include "utility" namespace nearby { namespace connections { @@ -53,11 +61,14 @@ class WifiDirectBwuHandler : public BaseBwuHandler { // WFD protocol to established connection while WINRT follow the standard WFD // spec to achieve the connection. So return fail to stop the upgrade request // from phone side. - std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) final; - Medium GetUpgradeMedium() const final { return Medium::WIFI_DIRECT; } + ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) final; + location::nearby::proto::connections::Medium GetUpgradeMedium() const final { + return location::nearby::proto::connections::Medium::WIFI_DIRECT; + } void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) final {} diff --git a/connections/implementation/wifi_direct_bwu_test.cc b/connections/implementation/wifi_direct_bwu_test.cc index b01d99981e..04d954128f 100644 --- a/connections/implementation/wifi_direct_bwu_test.cc +++ b/connections/implementation/wifi_direct_bwu_test.cc @@ -16,16 +16,28 @@ #include #include "gtest/gtest.h" +#include "absl/time/time.h" #include "connections/implementation/bwu_handler.h" +#include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" +#include "connections/implementation/mediums/mediums.h" +#include "connections/implementation/offline_frames.h" #include "connections/implementation/wifi_direct_bwu_handler.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/count_down_latch.h" +#include "internal/platform/exception.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" +#include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" +#include "internal/platform/single_thread_executor.h" namespace nearby { namespace connections { namespace { using ::location::nearby::connections::OfflineFrame; +using ::location::nearby::proto::connections::OperationResultCode; constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); } // namespace @@ -97,17 +109,22 @@ TEST_F(WifiDirectTest, WFDGOBWUInit_GCCreateEndpointChannel) { auto bwu_frame = upgrade_frame.result().v1().bandwidth_upgrade_negotiation(); - std::unique_ptr new_channel = + ErrorOr> result = handler_2->CreateUpgradedEndpointChannel( &wifi_direct_gc, /*service_id=*/"A", /*endpoint_id=*/"1", bwu_frame.upgrade_path_info()); if (!FeatureFlags::GetInstance().GetFlags().enable_cancellation_flag) { + ASSERT_TRUE(result.has_value()); + std::unique_ptr new_channel = std::move(result.value()); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); EXPECT_EQ(new_channel->GetMedium(), location::nearby::proto::connections::Medium::WIFI_DIRECT); } else { + EXPECT_FALSE(result.has_value()); + EXPECT_TRUE(result.has_error()); + EXPECT_EQ(result.error().operation_result_code(), + OperationResultCode::DETAIL_UNKNOWN); accept_latch.CountDown(); - EXPECT_EQ(new_channel, nullptr); } EXPECT_TRUE(mediums_2.GetWifiDirect().IsConnectedToGO()); handler_2->RevertResponderState(/*service_id=*/"A"); diff --git a/connections/implementation/wifi_hotspot_bwu_handler.cc b/connections/implementation/wifi_hotspot_bwu_handler.cc index 4e8b124716..43171578e1 100644 --- a/connections/implementation/wifi_hotspot_bwu_handler.cc +++ b/connections/implementation/wifi_hotspot_bwu_handler.cc @@ -15,7 +15,6 @@ #include "connections/implementation/wifi_hotspot_bwu_handler.h" #include -#include #include #include #include @@ -25,11 +24,11 @@ #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel.h" #include "connections/implementation/mediums/mediums.h" -#include "connections/implementation/mediums/utils.h" #include "connections/implementation/offline_frames.h" #include "connections/implementation/wifi_hotspot_endpoint_channel.h" #include "connections/strategy.h" #include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" #include "internal/platform/logging.h" #include "internal/platform/wifi_credential.h" #include "internal/platform/wifi_hotspot.h" @@ -37,6 +36,11 @@ namespace nearby { namespace connections { +namespace { +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace + +// TODO(edwinwu): Add exact OperationResultCode for WifiHotspotBwuHandler. WifiHotspotBwuHandler::WifiHotspotBwuHandler( Mediums& mediums, IncomingConnectionCallback incoming_connection_callback) : BaseBwuHandler(std::move(incoming_connection_callback)), @@ -108,13 +112,13 @@ void WifiHotspotBwuHandler::HandleRevertInitiatorStateForService( // Called by BWU target. Retrieves a new medium info from incoming message, // and establishes connection over WifiHotspot using this info. -std::unique_ptr +ErrorOr> WifiHotspotBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { if (!upgrade_path_info.has_wifi_hotspot_credentials()) { NEARBY_LOGS(INFO) << "No Hotspot Credential"; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } const UpgradePathInfo::WifiHotspotCredentials& upgrade_path_info_credentials = upgrade_path_info.wifi_hotspot_credentials(); @@ -131,7 +135,7 @@ WifiHotspotBwuHandler::CreateUpgradedEndpointChannel( if (!wifi_hotspot_medium_.ConnectWifiHotspot(ssid, password, frequency)) { NEARBY_LOGS(ERROR) << "Connect to Hotspot failed"; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } WifiHotspotSocket socket = wifi_hotspot_medium_.Connect( @@ -140,7 +144,7 @@ WifiHotspotBwuHandler::CreateUpgradedEndpointChannel( NEARBY_LOGS(ERROR) << "WifiHotspotBwuHandler failed to connect to the WifiHotspot service(" << gateway << ":" << port << ") for endpoint " << endpoint_id; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } NEARBY_VLOG(1) @@ -151,7 +155,7 @@ WifiHotspotBwuHandler::CreateUpgradedEndpointChannel( auto channel = std::make_unique( service_id, /*channel_name=*/service_id, socket); - return channel; + return {std::move(channel)}; } // Accept Connection Callback. diff --git a/connections/implementation/wifi_hotspot_bwu_handler.h b/connections/implementation/wifi_hotspot_bwu_handler.h index 3e4fd611ce..f19f708d7e 100644 --- a/connections/implementation/wifi_hotspot_bwu_handler.h +++ b/connections/implementation/wifi_hotspot_bwu_handler.h @@ -15,12 +15,19 @@ #ifndef CORE_INTERNAL_WIFI_HOTSPOT_BWU_HANDLER_H_ #define CORE_INTERNAL_WIFI_HOTSPOT_BWU_HANDLER_H_ +#include #include +#include #include "connections/implementation/base_bwu_handler.h" +#include "connections/implementation/bwu_handler.h" #include "connections/implementation/client_proxy.h" -#include "connections/implementation/endpoint_channel_manager.h" +#include "connections/implementation/endpoint_channel.h" #include "connections/implementation/mediums/mediums.h" +#include "connections/implementation/mediums/wifi_hotspot.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" +#include "internal/platform/wifi_hotspot.h" namespace nearby { namespace connections { @@ -49,11 +56,14 @@ class WifiHotspotBwuHandler : public BaseBwuHandler { }; // BwuHandler implementation: - std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) final; - Medium GetUpgradeMedium() const final { return Medium::WIFI_HOTSPOT; } + ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) final; + location::nearby::proto::connections::Medium GetUpgradeMedium() const final { + return location::nearby::proto::connections::Medium::WIFI_HOTSPOT; + } void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) final {} diff --git a/connections/implementation/wifi_hotspot_bwu_test.cc b/connections/implementation/wifi_hotspot_bwu_test.cc index 091087c366..44a4733331 100644 --- a/connections/implementation/wifi_hotspot_bwu_test.cc +++ b/connections/implementation/wifi_hotspot_bwu_test.cc @@ -12,19 +12,32 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include "gtest/gtest.h" +#include "absl/time/time.h" #include "connections/implementation/bwu_handler.h" +#include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" +#include "connections/implementation/mediums/mediums.h" +#include "connections/implementation/offline_frames.h" #include "connections/implementation/wifi_hotspot_bwu_handler.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/count_down_latch.h" +#include "internal/platform/exception.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" +#include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" +#include "internal/platform/single_thread_executor.h" namespace nearby { namespace connections { namespace { using ::location::nearby::connections::OfflineFrame; +using ::location::nearby::proto::connections::OperationResultCode; constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); } // namespace @@ -91,17 +104,22 @@ TEST_F(WifiHotspotTest, SoftAPBWUInit_STACreateEndpointChannel) { auto bwu_frame = upgrade_frame.result().v1().bandwidth_upgrade_negotiation(); - std::unique_ptr new_channel = + ErrorOr> result = handler_2->CreateUpgradedEndpointChannel(&client_2, /*service_id=*/"A", /*endpoint_id=*/"1", bwu_frame.upgrade_path_info()); if (!FeatureFlags::GetInstance().GetFlags().enable_cancellation_flag) { + ASSERT_TRUE(result.has_value()); + std::unique_ptr new_channel = std::move(result.value()); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); EXPECT_EQ(new_channel->GetMedium(), location::nearby::proto::connections::Medium::WIFI_HOTSPOT); } else { + EXPECT_FALSE(result.has_value()); + EXPECT_TRUE(result.has_error()); + EXPECT_EQ(result.error().operation_result_code(), + OperationResultCode::DETAIL_UNKNOWN); accept_latch.CountDown(); - EXPECT_EQ(new_channel, nullptr); } EXPECT_TRUE(mediums_2.GetWifiHotspot().IsConnectedToHotspot()); handler_2->RevertResponderState(/*service_id=*/"A"); diff --git a/connections/implementation/wifi_lan_bwu_handler.cc b/connections/implementation/wifi_lan_bwu_handler.cc index b1cb365065..04c751f62b 100644 --- a/connections/implementation/wifi_lan_bwu_handler.cc +++ b/connections/implementation/wifi_lan_bwu_handler.cc @@ -14,13 +14,20 @@ #include "connections/implementation/wifi_lan_bwu_handler.h" +#include +#include #include #include #include "absl/functional/bind_front.h" +#include "connections/implementation/base_bwu_handler.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" +#include "connections/implementation/mediums/mediums.h" #include "connections/implementation/offline_frames.h" #include "connections/implementation/wifi_lan_endpoint_channel.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" #include "internal/platform/implementation/wifi_utils.h" #include "internal/platform/logging.h" #include "internal/platform/wifi_lan.h" @@ -28,6 +35,11 @@ namespace nearby { namespace connections { +namespace { +using ::location::nearby::proto::connections::OperationResultCode; +} // namespace + +// TODO(edwinwu): Add exact OperationResultCode for WifiLanBwuHandler. WifiLanBwuHandler::WifiLanBwuHandler( Mediums& mediums, IncomingConnectionCallback incoming_connection_callback) : BaseBwuHandler(std::move(incoming_connection_callback)), @@ -35,19 +47,19 @@ WifiLanBwuHandler::WifiLanBwuHandler( // Called by BWU target. Retrieves a new medium info from incoming message, // and establishes connection over WifiLan using this info. -std::unique_ptr +ErrorOr> WifiLanBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { if (!upgrade_path_info.has_wifi_lan_socket()) { - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } const UpgradePathInfo::WifiLanSocket& upgrade_path_info_socket = upgrade_path_info.wifi_lan_socket(); if (!upgrade_path_info_socket.has_ip_address() || !upgrade_path_info_socket.has_wifi_port()) { NEARBY_LOGS(ERROR) << "WifiLanBwuHandler failed to parse UpgradePathInfo."; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } const std::string& ip_address = upgrade_path_info_socket.ip_address(); @@ -64,7 +76,7 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( << "WifiLanBwuHandler failed to connect to the WifiLan service (" << WifiUtils::GetHumanReadableIpAddress(ip_address) << ":" << port << ") for endpoint " << endpoint_id; - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } NEARBY_VLOG(1) @@ -80,10 +92,10 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( << "channel to the WifiLan service (" << ip_address << ":" << port << ") for endpoint " << endpoint_id; socket.Close(); - return nullptr; + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } - return channel; + return {std::move(channel)}; } // Called by BWU initiator. Set up WifiLan upgraded medium for this endpoint, @@ -143,12 +155,12 @@ void WifiLanBwuHandler::HandleRevertInitiatorStateForService( void WifiLanBwuHandler::OnIncomingWifiLanConnection( ClientProxy* client, const std::string& upgrade_service_id, WifiLanSocket socket) { - auto channel = absl::make_unique( + auto channel = std::make_unique( upgrade_service_id, /*channel_name=*/upgrade_service_id, socket); std::unique_ptr connection( new IncomingSocketConnection{ - .socket = absl::make_unique(upgrade_service_id, - socket), + .socket = std::make_unique(upgrade_service_id, + socket), .channel = std::move(channel), }); NotifyOnIncomingConnection(client, std::move(connection)); diff --git a/connections/implementation/wifi_lan_bwu_handler.h b/connections/implementation/wifi_lan_bwu_handler.h index aee1ae01d0..64bad31035 100644 --- a/connections/implementation/wifi_lan_bwu_handler.h +++ b/connections/implementation/wifi_lan_bwu_handler.h @@ -15,12 +15,19 @@ #ifndef CORE_INTERNAL_WIFI_LAN_BWU_HANDLER_H_ #define CORE_INTERNAL_WIFI_LAN_BWU_HANDLER_H_ +#include #include +#include #include "connections/implementation/base_bwu_handler.h" +#include "connections/implementation/bwu_handler.h" #include "connections/implementation/client_proxy.h" -#include "connections/implementation/endpoint_channel_manager.h" +#include "connections/implementation/endpoint_channel.h" #include "connections/implementation/mediums/mediums.h" +#include "connections/implementation/mediums/wifi_lan.h" +#include "internal/platform/byte_array.h" +#include "internal/platform/expected.h" +#include "internal/platform/wifi_lan.h" namespace nearby { namespace connections { @@ -49,11 +56,14 @@ class WifiLanBwuHandler : public BaseBwuHandler { }; // BwuHandler implementation: - std::unique_ptr CreateUpgradedEndpointChannel( - ClientProxy* client, const std::string& service_id, - const std::string& endpoint_id, - const UpgradePathInfo& upgrade_path_info) final; - Medium GetUpgradeMedium() const final { return Medium::WIFI_LAN; } + ErrorOr> + CreateUpgradedEndpointChannel(ClientProxy* client, + const std::string& service_id, + const std::string& endpoint_id, + const UpgradePathInfo& upgrade_path_info) final; + location::nearby::proto::connections::Medium GetUpgradeMedium() const final { + return location::nearby::proto::connections::Medium::WIFI_LAN; + } void OnEndpointDisconnect(ClientProxy* client, const std::string& endpoint_id) final {}