Translate API closes to Application closes on the wire.

Summary: Previously we would end up writing a non-application close when the application calls close(folly::none). This isn't correct, as those cases should be an application error with no error.

Reviewed By: afrind

Differential Revision: D20518529

fbshipit-source-id: fe069cccc32bd550fb3ec599694110955816993f

quic/QuicException.cpp

@@ -178,6 +178,10 @@ std::string cryptoErrorToString(TransportErrorCode code) {
 std::string toString(QuicErrorCode code) {
   switch (code.type()) {
     case QuicErrorCode::Type::ApplicationErrorCode_E:
+      if (*code.asApplicationErrorCode() ==
+          GenericApplicationErrorCode::NO_ERROR) {
+        return "No Error";
+      }
       return folly::to<std::string>(*code.asApplicationErrorCode());
     case QuicErrorCode::Type::LocalErrorCode_E:
       return toString(*code.asLocalErrorCode()).str();

quic/api/QuicTransportBase.cpp

@@ -147,6 +147,13 @@ void QuicTransportBase::close(
   // explicitly called close.
   connCallback_ = nullptr;
 
+  // If we were called with no error code, ensure that we are going to write
+  // an application close, so the peer knows it didn't come from the transport.
+  if (!errorCode) {
+    errorCode = std::make_pair(
+        GenericApplicationErrorCode::NO_ERROR,
+        toString(GenericApplicationErrorCode::NO_ERROR));
+  }
   closeImpl(std::move(errorCode), true);
   conn_->logger.reset();
 }
@@ -156,6 +163,11 @@ void QuicTransportBase::closeNow(
   DCHECK(getEventBase() && getEventBase()->isInEventBaseThread());
   FOLLY_MAYBE_UNUSED auto self = sharedGuard();
   VLOG(4) << __func__ << " " << *this;
+  if (!errorCode) {
+    errorCode = std::make_pair(
+        GenericApplicationErrorCode::NO_ERROR,
+        toString(GenericApplicationErrorCode::NO_ERROR));
+  }
   closeImpl(std::move(errorCode), false);
   // the drain timeout may have been scheduled by a previous close, in which
   // case, our close would not take effect. This cancels the drain timeout in

quic/api/test/QuicTransportBaseTest.cpp

@@ -1243,7 +1243,9 @@ TEST_F(QuicTransportImplTest, RegisterDeliveryCallbackLowerThanExpectedClose) {
 
 TEST_F(QuicTransportImplTest, TestNotifyPendingConnWriteOnCloseWithoutError) {
   NiceMock<MockWriteCallback> wcb;
-  EXPECT_CALL(wcb, onConnectionWriteError(IsError(LocalErrorCode::NO_ERROR)));
+  EXPECT_CALL(
+      wcb,
+      onConnectionWriteError(IsError(GenericApplicationErrorCode::NO_ERROR)));
   transport->notifyPendingWriteOnConnection(&wcb);
   transport->close(folly::none);
   evb->loopOnce();
@@ -1281,7 +1283,9 @@ TEST_F(QuicTransportImplTest, TestNotifyPendingWriteOnCloseWithoutError) {
   auto stream = transport->createBidirectionalStream().value();
   NiceMock<MockWriteCallback> wcb;
   EXPECT_CALL(
-      wcb, onStreamWriteError(stream, IsError(LocalErrorCode::NO_ERROR)));
+      wcb,
+      onStreamWriteError(
+          stream, IsError(GenericApplicationErrorCode::NO_ERROR)));
   transport->notifyPendingWriteOnStream(stream, &wcb);
   transport->close(folly::none);
   evb->loopOnce();
@@ -1369,7 +1373,8 @@ TEST_F(QuicTransportImplTest, TestGracefulCloseWithNoActiveStream) {
   NiceMock<MockWriteCallback> wcbConn;
   NiceMock<MockReadCallback> rcb;
   NiceMock<MockDeliveryCallback> deliveryCb;
-  EXPECT_CALL(rcb, readError(stream, IsError(LocalErrorCode::NO_ERROR)));
+  EXPECT_CALL(
+      rcb, readError(stream, IsError(GenericApplicationErrorCode::NO_ERROR)));
   EXPECT_CALL(deliveryCb, onDeliveryAck(stream, _, _));
 
   EXPECT_CALL(connCallback, onConnectionEnd()).Times(0);

quic/client/test/QuicClientTransportTest.cpp

@@ -1862,7 +1862,7 @@ TEST_F(QuicClientTransportTest, SocketClosedDuringOnTransportReady) {
   setupCryptoLayer();
   client->start(&callback);
   setConnectionIds();
-  recvServerHello();
+  EXPECT_THROW(recvServerHello(), std::runtime_error);
 }
 
 TEST_F(QuicClientTransportTest, NetworkUnreachableIsFatalToConn) {
@@ -3597,10 +3597,8 @@ TEST_P(
     EXPECT_EQ(indices.size(), 1);
     auto tmp = std::move(qLogger->logs[indices[0]]);
     auto event = dynamic_cast<QLogConnectionCloseEvent*>(tmp.get());
-    EXPECT_EQ(event->error, kNoError);
+    EXPECT_EQ(event->error, "No Error");
-    auto reason = folly::to<std::string>(
+    EXPECT_EQ(event->reason, "No Error");
-        "Server: ", kNoError, ", Peer: isReset: ", 0, ", Peer: isAbandon: ", 0);
-    EXPECT_EQ(event->reason, reason);
     EXPECT_TRUE(event->drainConnection);
     EXPECT_TRUE(event->sendCloseImmediately);
   }

quic/server/test/QuicServerTransportTest.cpp

@@ -1669,7 +1669,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterReset) {
       .WillOnce(Invoke([&](StreamId /*sid*/, ApplicationErrorCode /*e*/) {
         server->close(folly::none);
       }));
-  deliverData(packetToBuf(packet));
+  EXPECT_THROW(deliverData(packetToBuf(packet)), std::runtime_error);
 }
 
 TEST_F(QuicServerTransportTest, StopSendingLoss) {