From 28e21906355b00a8d35e8de2cb4f9833337c4d0b Mon Sep 17 00:00:00 2001 From: John Knox Date: Fri, 30 Nov 2018 07:02:33 -0800 Subject: [PATCH] Improve "Connect to desktop" diagnostic step Summary: This does two things: 1. Change "Connect to desktop" to say which connection it's establishing instead, which makes it clearer what's actually happening. This step really does mean establishing a connection, because what happens during that connection is asynchronous after this step completes. 2. Call step->complete() after certificate exchange connection is established. This wasn't happening before, so you'd always get "[Failed] Connect to desktop" even after a successful cert exchange. Reviewed By: priteshrnandgaonkar Differential Revision: D13256783 fbshipit-source-id: 5e8e3a54f52d2e0adbde4c6d82d1acc840f1eb59 --- .../Flipper/FlipperConnectionManagerImpl.cpp | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/xplat/Flipper/FlipperConnectionManagerImpl.cpp b/xplat/Flipper/FlipperConnectionManagerImpl.cpp index 8fc7af78c..94a8d1d7d 100644 --- a/xplat/Flipper/FlipperConnectionManagerImpl.cpp +++ b/xplat/Flipper/FlipperConnectionManagerImpl.cpp @@ -1,9 +1,8 @@ -/* - * Copyright (c) Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ #include "FlipperConnectionManagerImpl.h" #include @@ -117,29 +116,31 @@ void FlipperConnectionManagerImpl::startSync() { log("Already connected"); return; } - auto connect = flipperState_->start("Connect to desktop"); + bool isClientSetupStep = isCertificateExchangeNeeded(); + auto step = flipperState_->start( + isClientSetupStep ? "Establish pre-setup connection" + : "Establish main connection"); try { - if (isCertificateExchangeNeeded()) { + if (isClientSetupStep) { doCertificateExchange(); - return; + } else { + connectSecurely(); } - - connectSecurely(); - connect->complete(); + step->complete(); } catch (const folly::AsyncSocketException& e) { if (e.getType() == folly::AsyncSocketException::NOT_OPEN) { // The expected code path when flipper desktop is not running. // Don't count as a failed attempt. - connect->fail("Port not open"); + step->fail("Port not open"); } else { log(e.what()); failedConnectionAttempts_++; - connect->fail(e.what()); + step->fail(e.what()); } reconnect(); } catch (const std::exception& e) { log(e.what()); - connect->fail(e.what()); + step->fail(e.what()); failedConnectionAttempts_++; reconnect(); }