Add a AsyncSocket::LifecycleObserver->connectError() callback

Summary:
The LifecycleObserver already tracks connect() but does not notify on
connectError().  Follow the same pattern.

Note that we intentionally added this to AsyncSocket's LifecycleObserver
rather than AsyncTransport b/c AsyncTransport is not supposed to have
a concept of a 'connection'.  This is counter to the 'connect()' callback
that already exists in AsyncTransport::LifecycleObserver so marked a quick
TODO to remember to move that.

Reviewed By: bschlinker

Differential Revision: D28612132

fbshipit-source-id: cefa650bd007d48e38f9cb67bc9a82d322036d4c

folly/io/async/AsyncSocket.cpp

@@ -3630,6 +3630,14 @@ void AsyncSocket::invalidState(ConnectCallback* callback) {
       AsyncSocketException::ALREADY_OPEN,
       "connect() called with socket in invalid state");
   connectEndTime_ = std::chrono::steady_clock::now();
+  if ((state_ == StateEnum::CONNECTING) || (state_ == StateEnum::ERROR)) {
+    for (const auto& cb : lifecycleObservers_) {
+      if (auto observer = dynamic_cast<AsyncSocket::LifecycleObserver*>(cb)) {
+        // inform any lifecycle observes that the connection failed
+        observer->connectError(this, ex);
+      }
+    }
+  }
   if (state_ == StateEnum::CLOSED || state_ == StateEnum::ERROR) {
     if (callback) {
       callback->connectErr(ex);
@@ -3673,6 +3681,18 @@ void AsyncSocket::invokeConnectErr(const AsyncSocketException& ex) {
   VLOG(5) << "AsyncSocket(this=" << this << ", fd=" << fd_
           << "): connect err invoked with ex: " << ex.what();
   connectEndTime_ = std::chrono::steady_clock::now();
+  if ((state_ == StateEnum::CONNECTING) || (state_ == StateEnum::ERROR)) {
+    // invokeConnectErr() can be invoked when state is {FAST_OPEN, CLOSED,
+    // ESTABLISHED} (!?) and a bunch of other places that are not what this call
+    // back wants. This seems like a bug but work around here while we explore
+    // it independently
+    for (const auto& cb : lifecycleObservers_) {
+      if (auto observer = dynamic_cast<AsyncSocket::LifecycleObserver*>(cb)) {
+        // inform any lifecycle observes that the connection failed
+        observer->connectError(this, ex);
+      }
+    }
+  }
   if (connectCallback_) {
     ConnectCallback* callback = connectCallback_;
     connectCallback_ = nullptr;

folly/io/async/AsyncTransport.h

@@ -1008,6 +1008,18 @@ class AsyncTransport : public DelayedDestruction,
      */
     virtual void connect(AsyncTransport* /* transport */) noexcept = 0;
 
+    /**
+     * connectError() will be invoked when connect() returns an error.
+     *
+     * Triggered before any application connection callback.
+     *
+     * @param transport   Transport that has connected.
+     * @param ex          Exception that describes why.
+     */
+    virtual void connectError(
+        AsyncTransport* /* transport */,
+        const AsyncSocketException& /* ex */) noexcept {}
+
     /**
      * Invoked when the transport is being attached to an EventBase.
      *

folly/io/async/test/AsyncSocketTest2.cpp

@@ -7588,6 +7588,32 @@ TEST(AsyncSocket, LifecycleObserverAttachThenDestroySocket) {
   Mock::VerifyAndClearExpectations(cb.get());
 }
 
+TEST(AsyncSocket, LifecycleObserverAttachThenConnectError) {
+  auto cb = std::make_unique<StrictMock<MockAsyncSocketLifecycleObserver>>();
+  // port =1 is unreachble on localhost
+  folly::SocketAddress unreachable{"::1", 1};
+
+  EventBase evb;
+  auto socket = AsyncSocket::UniquePtr(new AsyncSocket(&evb));
+  EXPECT_CALL(*cb, observerAttachMock(socket.get()));
+  socket->addLifecycleObserver(cb.get());
+  EXPECT_THAT(socket->getLifecycleObservers(), UnorderedElementsAre(cb.get()));
+  Mock::VerifyAndClearExpectations(cb.get());
+
+  // the current state machine calls AsyncSocket::invokeConnectionError() twice
+  // for this use-case...
+  EXPECT_CALL(*cb, fdAttachMock(socket.get()));
+  EXPECT_CALL(*cb, connectErrorMock(socket.get(), _)).Times(2);
+  EXPECT_CALL(*cb, closeMock(socket.get()));
+  socket->connect(nullptr, unreachable, 1);
+  evb.loop();
+  Mock::VerifyAndClearExpectations(cb.get());
+
+  EXPECT_CALL(*cb, destroyMock(socket.get()));
+  socket = nullptr;
+  Mock::VerifyAndClearExpectations(cb.get());
+}
+
 TEST(AsyncSocket, LifecycleObserverMultipleAttachThenDestroySocket) {
   auto cb1 = std::make_unique<StrictMock<MockAsyncSocketLifecycleObserver>>();
   auto cb2 = std::make_unique<StrictMock<MockAsyncSocketLifecycleObserver>>();

folly/io/async/test/MockAsyncSocketObserver.h

@@ -17,6 +17,7 @@
 #pragma once
 
 #include <folly/io/async/AsyncSocket.h>
+#include <folly/io/async/AsyncSocketException.h>
 #include <folly/portability/GMock.h>
 
 namespace folly {
@@ -37,6 +38,8 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver {
   MOCK_METHOD1(destroyMock, void(AsyncTransport*));
   MOCK_METHOD1(closeMock, void(AsyncTransport*));
   MOCK_METHOD1(connectMock, void(AsyncTransport*));
+  MOCK_METHOD2(
+      connectErrorMock, void(AsyncTransport*, const AsyncSocketException&));
   MOCK_METHOD2(evbAttachMock, void(AsyncTransport*, EventBase*));
   MOCK_METHOD2(evbDetachMock, void(AsyncTransport*, EventBase*));
   MOCK_METHOD2(
@@ -61,6 +64,10 @@ class MockAsyncSocketLifecycleObserver : public AsyncSocket::LifecycleObserver {
   void destroy(AsyncTransport* trans) noexcept override { destroyMock(trans); }
   void close(AsyncTransport* trans) noexcept override { closeMock(trans); }
   void connect(AsyncTransport* trans) noexcept override { connectMock(trans); }
+  void connectError(
+      AsyncTransport* trans, const AsyncSocketException& ex) noexcept override {
+    connectErrorMock(trans, ex);
+  }
   void evbAttach(AsyncTransport* trans, EventBase* eb) noexcept override {
     evbAttachMock(trans, eb);
   }