Merge Application Close Frame and Connection Close Frame

Summary: As a part of Draft 17, application close frame has been removed, we use connection close frame to represent both application close and connection close.

Reviewed By: mjoras

Differential Revision: D18580856

fbshipit-source-id: d274fa2d3dbc59b926bca5a2b8a20328ae582703

quic/QuicConstants.h

@@ -94,7 +94,8 @@ enum class FrameType : uint8_t {
   PATH_CHALLENGE = 0x1A,
   PATH_RESPONSE = 0x1B,
   CONNECTION_CLOSE = 0x1C,
-  APPLICATION_CLOSE = 0x1D,
+  // CONNECTION_CLOSE_APP_ERR frametype is use to indicate application errors
+  CONNECTION_CLOSE_APP_ERR = 0x1D,
   MIN_STREAM_DATA = 0xFE, // subject to change (https://fburl.com/qpr)
   EXPIRED_STREAM_DATA = 0xFF, // subject to change (https://fburl.com/qpr)
 };

quic/api/QuicTransportFunctions.cpp

@@ -743,29 +743,33 @@ void writeCloseCommon(
   if (!closeDetails) {
     written = writeFrame(
         ConnectionCloseFrame(
-            TransportErrorCode::NO_ERROR, std::string("No error")),
+            QuicErrorCode(TransportErrorCode::NO_ERROR),
+            std::string("No error")),
         packetBuilder);
   } else {
     switch (closeDetails->first.type()) {
       case QuicErrorCode::Type::ApplicationErrorCode_E:
         written = writeFrame(
-            ApplicationCloseFrame(
+            ConnectionCloseFrame(
-                *closeDetails->first.asApplicationErrorCode(),
+                QuicErrorCode(*closeDetails->first.asApplicationErrorCode()),
-                closeDetails->second),
+                closeDetails->second,
+                quic::FrameType::CONNECTION_CLOSE_APP_ERR),
             packetBuilder);
         break;
       case QuicErrorCode::Type::TransportErrorCode_E:
         written = writeFrame(
             ConnectionCloseFrame(
-                *closeDetails->first.asTransportErrorCode(),
+                QuicErrorCode(*closeDetails->first.asTransportErrorCode()),
-                closeDetails->second),
+                closeDetails->second,
+                quic::FrameType::CONNECTION_CLOSE),
             packetBuilder);
         break;
       case QuicErrorCode::Type::LocalErrorCode_E:
         written = writeFrame(
             ConnectionCloseFrame(
-                TransportErrorCode::INTERNAL_ERROR,
+                QuicErrorCode(TransportErrorCode::INTERNAL_ERROR),
-                std::string("Internal error")),
+                std::string("Internal error"),
+                quic::FrameType::CONNECTION_CLOSE),
             packetBuilder);
         break;
     }

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -364,7 +364,8 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
 
   // Create few frames
   ConnectionCloseFrame connCloseFrame(
-      TransportErrorCode::FRAME_ENCODING_ERROR, "The sun is in the sky.");
+      QuicErrorCode(TransportErrorCode::FRAME_ENCODING_ERROR),
+      "The sun is in the sky.");
   MaxStreamsFrame maxStreamFrame(999, true);
   PingFrame pingFrame;
   IntervalSet<PacketNum> ackBlocks;

quic/client/QuicClientTransport.cpp

@@ -486,21 +486,6 @@ void QuicClientTransport::processPacketData(
             "Peer closed", TransportErrorCode::NO_ERROR);
         break;
       }
-      case QuicFrame::Type::ApplicationCloseFrame_E: {
-        ApplicationCloseFrame& appClose = *quicFrame.asApplicationCloseFrame();
-        auto errMsg = folly::to<std::string>(
-            "Client closed by peer reason=", appClose.reasonPhrase);
-        VLOG(4) << errMsg << " " << *this;
-        if (conn_->qLogger) {
-          conn_->qLogger->addTransportStateUpdate(getPeerClose(errMsg));
-        }
-        QUIC_TRACE(recvd_close, *conn_, errMsg.c_str());
-        conn_->peerConnectionError = std::make_pair(
-            QuicErrorCode(appClose.errorCode), std::move(errMsg));
-        throw QuicTransportException(
-            "Peer closed", TransportErrorCode::NO_ERROR);
-        break;
-      }
       case QuicFrame::Type::PaddingFrame_E: {
         break;
       }

quic/client/test/QuicClientTransportTest.cpp

@@ -3207,7 +3207,7 @@ TEST_P(
     EXPECT_TRUE(verifyFramePresent(
         socketWrites,
         *makeHandshakeCodec(),
-        QuicFrame::Type::ApplicationCloseFrame_E));
+        QuicFrame::Type::ConnectionCloseFrame_E));
 
     std::vector<int> indices =
         getQLogEventIndices(QLogEventType::ConnectionClose, qLogger);
@@ -3376,7 +3376,7 @@ TEST_P(QuicClientTransportAfterStartTestClose, CloseConnectionWithError) {
     EXPECT_TRUE(verifyFramePresent(
         socketWrites,
         *makeEncryptedCodec(),
-        QuicFrame::Type::ApplicationCloseFrame_E));
+        QuicFrame::Type::ConnectionCloseFrame_E));
   } else {
     client->close(folly::none);
     EXPECT_TRUE(verifyFramePresent(
@@ -3589,7 +3589,7 @@ TEST_F(QuicClientTransportAfterStartTest, IdleTimeoutExpired) {
   auto serverCodec = makeEncryptedCodec();
   // We expect a conn close in a cleartext packet.
   EXPECT_FALSE(verifyFramePresent(
-      socketWrites, *serverCodec, QuicFrame::Type::ApplicationCloseFrame_E));
+      socketWrites, *serverCodec, QuicFrame::Type::ConnectionCloseFrame_E));
   EXPECT_FALSE(verifyFramePresent(
       socketWrites, *serverCodec, QuicFrame::Type::ConnectionCloseFrame_E));
   EXPECT_TRUE(socketWrites.empty());
@@ -4520,7 +4520,8 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveConnectionClose) {
       0,
       QuicVersion::MVFST);
   ConnectionCloseFrame connClose(
-      TransportErrorCode::NO_ERROR, "Stand clear of the closing doors, please");
+      QuicErrorCode(TransportErrorCode::NO_ERROR),
+      "Stand clear of the closing doors, please");
   writeFrame(std::move(connClose), builder);
   auto packet = packetToBuf(std::move(builder).buildPacket());
   EXPECT_CALL(clientConnCallback, onConnectionEnd());
@@ -4547,8 +4548,8 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveApplicationClose) {
       std::move(header),
       0,
       QuicVersion::MVFST);
-  ApplicationCloseFrame appClose(
+  ConnectionCloseFrame appClose(
-      GenericApplicationErrorCode::UNKNOWN,
+      QuicErrorCode(GenericApplicationErrorCode::UNKNOWN),
       "Stand clear of the closing doors, please");
   writeFrame(std::move(appClose), builder);
   auto packet = packetToBuf(std::move(builder).buildPacket());

quic/codec/Decode.cpp

@@ -582,10 +582,10 @@ ConnectionCloseFrame decodeConnectionCloseFrame(
   auto reasonPhrase =
       cursor.readFixedString(folly::to<size_t>(reasonPhraseLength->first));
   return ConnectionCloseFrame(
-      errorCode, std::move(reasonPhrase), triggeringFrameType);
+      QuicErrorCode(errorCode), std::move(reasonPhrase), triggeringFrameType);
 }
 
-ApplicationCloseFrame decodeApplicationCloseFrame(
+ConnectionCloseFrame decodeApplicationClose(
     folly::io::Cursor& cursor,
     const CodecParameters& params) {
   ApplicationErrorCode errorCode{};
@@ -600,9 +600,19 @@ ApplicationCloseFrame decodeApplicationCloseFrame(
       throw QuicTransportException(
           "Failed to parse error code.",
           quic::TransportErrorCode::FRAME_ENCODING_ERROR,
-          quic::FrameType::APPLICATION_CLOSE);
+          quic::FrameType::CONNECTION_CLOSE_APP_ERR);
     }
   }
+
+  auto frameTypeField = decodeQuicInteger(cursor);
+  if (UNLIKELY(!frameTypeField || frameTypeField->second != sizeof(uint8_t))) {
+    throw QuicTransportException(
+        "Bad connection close triggering frame type value",
+        quic::TransportErrorCode::FRAME_ENCODING_ERROR,
+        quic::FrameType::CONNECTION_CLOSE);
+  }
+  auto triggeringFrameType = static_cast<FrameType>(frameTypeField->first);
+
   auto reasonPhraseLength = decodeQuicInteger(cursor);
   if (UNLIKELY(
           !reasonPhraseLength ||
@@ -610,11 +620,13 @@ ApplicationCloseFrame decodeApplicationCloseFrame(
     throw QuicTransportException(
         "Bad reason phrase length",
         quic::TransportErrorCode::FRAME_ENCODING_ERROR,
-        quic::FrameType::APPLICATION_CLOSE);
+        quic::FrameType::CONNECTION_CLOSE_APP_ERR);
   }
+
   auto reasonPhrase =
       cursor.readFixedString(folly::to<size_t>(reasonPhraseLength->first));
-  return ApplicationCloseFrame(errorCode, std::move(reasonPhrase));
+  return ConnectionCloseFrame(
+      QuicErrorCode(errorCode), std::move(reasonPhrase), triggeringFrameType);
 }
 
 MinStreamDataFrame decodeMinStreamDataFrame(folly::io::Cursor& cursor) {
@@ -734,8 +746,8 @@ QuicFrame parseFrame(
         return QuicFrame(decodePathResponseFrame(cursor));
       case FrameType::CONNECTION_CLOSE:
         return QuicFrame(decodeConnectionCloseFrame(cursor, params));
-      case FrameType::APPLICATION_CLOSE:
+      case FrameType::CONNECTION_CLOSE_APP_ERR:
-        return QuicFrame(decodeApplicationCloseFrame(cursor, params));
+        return QuicFrame(decodeApplicationClose(cursor, params));
       case FrameType::MIN_STREAM_DATA:
         return QuicFrame(decodeMinStreamDataFrame(cursor));
       case FrameType::EXPIRED_STREAM_DATA:

quic/codec/Decode.h

@@ -82,7 +82,7 @@ ConnectionCloseFrame decodeConnectionCloseFrame(
     folly::io::Cursor& cursor,
     const CodecParameters& params);
 
-ApplicationCloseFrame decodeApplicationCloseFrame(
+ConnectionCloseFrame decodeApplicationClose(
     folly::io::Cursor& cursor,
     const CodecParameters& params);
 

quic/codec/QuicWriteCodec.cpp

@@ -619,13 +619,25 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
     case QuicWriteFrame::Type::ConnectionCloseFrame_E: {
       ConnectionCloseFrame& connectionCloseFrame =
           *frame.asConnectionCloseFrame();
-      QuicInteger intFrameType(
+      // Need to distinguish between CONNECTION_CLOSE & CONNECTINO_CLOSE_APP_ERR
-          static_cast<uint8_t>(FrameType::CONNECTION_CLOSE));
+      const TransportErrorCode* isTransportErrorCode =
+          connectionCloseFrame.errorCode.asTransportErrorCode();
+      const ApplicationErrorCode* isApplicationErrorCode =
+          connectionCloseFrame.errorCode.asApplicationErrorCode();
+
+      QuicInteger intFrameType(static_cast<uint8_t>(
+          isTransportErrorCode ? FrameType::CONNECTION_CLOSE
+                               : FrameType::CONNECTION_CLOSE_APP_ERR));
+
       QuicInteger reasonLength(connectionCloseFrame.reasonPhrase.size());
       QuicInteger closingFrameType(
           static_cast<FrameTypeType>(connectionCloseFrame.closingFrameType));
+
       QuicInteger errorCode(
-          static_cast<uint64_t>(connectionCloseFrame.errorCode));
+          isTransportErrorCode
+              ? static_cast<uint64_t>(TransportErrorCode(*isTransportErrorCode))
+              : static_cast<uint64_t>(
+                    ApplicationErrorCode(*isApplicationErrorCode)));
       auto version = builder.getVersion();
       size_t errorSize = version == QuicVersion::MVFST_OLD
           ? sizeof(TransportErrorCode)
@@ -637,8 +649,10 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
         builder.write(intFrameType);
         if (version == QuicVersion::MVFST_OLD) {
           builder.writeBE(
-              static_cast<std::underlying_type<TransportErrorCode>::type>(
+              isTransportErrorCode ? static_cast<uint16_t>(TransportErrorCode(
-                  connectionCloseFrame.errorCode));
+                                         *isTransportErrorCode))
+                                   : static_cast<uint16_t>(ApplicationErrorCode(
+                                         *isApplicationErrorCode)));
         } else {
           builder.write(errorCode);
         }
@@ -653,38 +667,6 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
       // no space left in packet
       return size_t(0);
     }
-    case QuicWriteFrame::Type::ApplicationCloseFrame_E: {
-      ApplicationCloseFrame& applicationCloseFrame =
-          *frame.asApplicationCloseFrame();
-      QuicInteger intFrameType(
-          static_cast<uint8_t>(FrameType::APPLICATION_CLOSE));
-      QuicInteger reasonLength(applicationCloseFrame.reasonPhrase.size());
-      QuicInteger errorCode(
-          static_cast<uint64_t>(applicationCloseFrame.errorCode));
-      auto version = builder.getVersion();
-      size_t errorSize = version == QuicVersion::MVFST_OLD
-          ? sizeof(ApplicationErrorCode)
-          : errorCode.getSize();
-      auto applicationCloseFrameSize = intFrameType.getSize() + errorSize +
-          reasonLength.getSize() + applicationCloseFrame.reasonPhrase.size();
-      if (packetSpaceCheck(spaceLeft, applicationCloseFrameSize)) {
-        builder.write(intFrameType);
-        if (version == QuicVersion::MVFST_OLD) {
-          builder.writeBE(static_cast<ApplicationErrorCode>(
-              applicationCloseFrame.errorCode));
-        } else {
-          builder.write(errorCode);
-        }
-        builder.write(reasonLength);
-        builder.push(
-            (const uint8_t*)applicationCloseFrame.reasonPhrase.data(),
-            applicationCloseFrame.reasonPhrase.size());
-        builder.appendFrame(std::move(applicationCloseFrame));
-        return applicationCloseFrameSize;
-      }
-      // no space left in packet
-      return size_t(0);
-    }
     case QuicWriteFrame::Type::QuicSimpleFrame_E: {
       return writeSimpleFrame(std::move(*frame.asQuicSimpleFrame()), builder);
     }

quic/codec/Types.cpp

@@ -442,7 +442,7 @@ std::string toString(FrameType frame) {
       return "PATH_RESPONSE";
     case FrameType::CONNECTION_CLOSE:
       return "CONNECTION_CLOSE";
-    case FrameType::APPLICATION_CLOSE:
+    case FrameType::CONNECTION_CLOSE_APP_ERR:
       return "APPLICATION_CLOSE";
     case FrameType::MIN_STREAM_DATA:
       return "MIN_STREAM_DATA";

quic/codec/Types.h

@@ -505,17 +505,17 @@ struct PathResponseFrame {
 
 struct ConnectionCloseFrame {
   // Members are not const to allow this to be movable.
-  TransportErrorCode errorCode;
+  QuicErrorCode errorCode;
   std::string reasonPhrase;
   // Per QUIC specification: type of frame that triggered the (close) error.
   // A value of 0 (PADDING frame) implies the frame type is unknown
   FrameType closingFrameType;
 
   ConnectionCloseFrame(
-      TransportErrorCode errorCodeIn,
+      QuicErrorCode errorCodeIn,
       std::string reasonPhraseIn,
       FrameType closingFrameTypeIn = FrameType::PADDING)
-      : errorCode(errorCodeIn),
+      : errorCode(std::move(errorCodeIn)),
         reasonPhrase(std::move(reasonPhraseIn)),
         closingFrameType(closingFrameTypeIn) {}
 
@@ -528,21 +528,6 @@ struct ConnectionCloseFrame {
   }
 };
 
-struct ApplicationCloseFrame {
-  // Members are not const to allow this to be movable.
-  ApplicationErrorCode errorCode;
-  std::string reasonPhrase;
-
-  ApplicationCloseFrame(
-      ApplicationErrorCode errorCodeIn,
-      std::string reasonPhraseIn)
-      : errorCode(errorCodeIn), reasonPhrase(std::move(reasonPhraseIn)) {}
-
-  bool operator==(const ApplicationCloseFrame& rhs) const {
-    return errorCode == rhs.errorCode && reasonPhrase == rhs.reasonPhrase;
-  }
-};
-
 // Frame to represent ones we skip
 struct NoopFrame {
   bool operator==(const NoopFrame&) const {
@@ -574,7 +559,6 @@ DECLARE_VARIANT_TYPE(QuicSimpleFrame, QUIC_SIMPLE_FRAME)
   F(PaddingFrame, __VA_ARGS__)           \
   F(RstStreamFrame, __VA_ARGS__)         \
   F(ConnectionCloseFrame, __VA_ARGS__)   \
-  F(ApplicationCloseFrame, __VA_ARGS__)  \
   F(MaxDataFrame, __VA_ARGS__)           \
   F(MaxStreamDataFrame, __VA_ARGS__)     \
   F(DataBlockedFrame, __VA_ARGS__)       \
@@ -593,7 +577,6 @@ DECLARE_VARIANT_TYPE(QuicFrame, QUIC_FRAME)
   F(PaddingFrame, __VA_ARGS__)           \
   F(RstStreamFrame, __VA_ARGS__)         \
   F(ConnectionCloseFrame, __VA_ARGS__)   \
-  F(ApplicationCloseFrame, __VA_ARGS__)  \
   F(MaxDataFrame, __VA_ARGS__)           \
   F(MaxStreamDataFrame, __VA_ARGS__)     \
   F(DataBlockedFrame, __VA_ARGS__)       \

quic/codec/test/QuicPacketRebuilderTest.cpp

@@ -59,7 +59,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
 
   // Get a bunch frames
   ConnectionCloseFrame connCloseFrame(
-      TransportErrorCode::FRAME_ENCODING_ERROR,
+      QuicErrorCode(TransportErrorCode::FRAME_ENCODING_ERROR),
       "The sun is in the sky.",
       FrameType::ACK);
   MaxStreamsFrame maxStreamsFrame(4321, true);
@@ -129,8 +129,10 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
       case QuicWriteFrame::Type::ConnectionCloseFrame_E: {
         const ConnectionCloseFrame& closeFrame =
             *frame.asConnectionCloseFrame();
+        const TransportErrorCode* transportErrorCode =
+            closeFrame.errorCode.asTransportErrorCode();
         EXPECT_EQ(
-            TransportErrorCode::FRAME_ENCODING_ERROR, closeFrame.errorCode);
+            TransportErrorCode::FRAME_ENCODING_ERROR, *transportErrorCode);
         EXPECT_EQ("The sun is in the sky.", closeFrame.reasonPhrase);
         EXPECT_EQ(FrameType::ACK, closeFrame.closingFrameType);
         break;
@@ -363,7 +365,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) {
 
   // Get a bunch frames
   ConnectionCloseFrame connCloseFrame(
-      TransportErrorCode::FRAME_ENCODING_ERROR,
+      QuicErrorCode(TransportErrorCode::FRAME_ENCODING_ERROR),
       "The sun is in the sky.",
       FrameType::ACK);
   StreamsBlockedFrame maxStreamIdFrame(0x1024, true);

quic/codec/test/QuicWriteCodecTest.cpp

@@ -1033,7 +1033,7 @@ TEST_F(QuicWriteCodecTest, WriteConnClose) {
   setupCommonExpects(pktBuilder);
   std::string reasonPhrase("You are fired");
   ConnectionCloseFrame connectionCloseFrame(
-      TransportErrorCode::PROTOCOL_VIOLATION, reasonPhrase);
+      QuicErrorCode(TransportErrorCode::PROTOCOL_VIOLATION), reasonPhrase);
   auto connCloseBytesWritten =
       writeFrame(std::move(connectionCloseFrame), pktBuilder);
 
@@ -1043,16 +1043,18 @@ TEST_F(QuicWriteCodecTest, WriteConnClose) {
   EXPECT_EQ(4 + reasonPhrase.size(), connCloseBytesWritten);
   auto& resultConnCloseFrame =
       *regularPacket.frames[0].asConnectionCloseFrame();
-  EXPECT_EQ(
+  const TransportErrorCode* transportErrorCode =
-      TransportErrorCode::PROTOCOL_VIOLATION, resultConnCloseFrame.errorCode);
+      resultConnCloseFrame.errorCode.asTransportErrorCode();
+  EXPECT_EQ(TransportErrorCode::PROTOCOL_VIOLATION, *transportErrorCode);
   EXPECT_EQ("You are fired", resultConnCloseFrame.reasonPhrase);
 
   auto wireBuf = std::move(builtOut.second);
   folly::io::Cursor cursor(wireBuf.get());
   QuicFrame decodedCloseFrame = parseQuicFrame(cursor);
   auto& wireConnCloseFrame = *decodedCloseFrame.asConnectionCloseFrame();
-  EXPECT_EQ(
+  const TransportErrorCode* protocolViolationCode =
-      TransportErrorCode::PROTOCOL_VIOLATION, wireConnCloseFrame.errorCode);
+      wireConnCloseFrame.errorCode.asTransportErrorCode();
+  EXPECT_EQ(TransportErrorCode::PROTOCOL_VIOLATION, *protocolViolationCode);
   EXPECT_EQ("You are fired", wireConnCloseFrame.reasonPhrase);
 
   // At last, verify there is nothing left in the wire format bytes:
@@ -1065,14 +1067,15 @@ TEST_F(QuicWriteCodecTest, DecodeConnCloseLarge) {
   std::string reasonPhrase;
   reasonPhrase.resize(kMaxReasonPhraseLength + 10);
   ConnectionCloseFrame connectionCloseFrame(
-      TransportErrorCode::PROTOCOL_VIOLATION, reasonPhrase);
+      QuicErrorCode(TransportErrorCode::PROTOCOL_VIOLATION), reasonPhrase);
   writeFrame(connectionCloseFrame, pktBuilder);
   auto builtOut = std::move(pktBuilder).buildPacket();
   auto regularPacket = builtOut.first;
   auto& resultConnCloseFrame =
       *regularPacket.frames[0].asConnectionCloseFrame();
-  EXPECT_EQ(
+  const TransportErrorCode* protocolViolationCode =
-      TransportErrorCode::PROTOCOL_VIOLATION, resultConnCloseFrame.errorCode);
+      resultConnCloseFrame.errorCode.asTransportErrorCode();
+  EXPECT_EQ(TransportErrorCode::PROTOCOL_VIOLATION, *protocolViolationCode);
   EXPECT_EQ(resultConnCloseFrame.reasonPhrase, reasonPhrase);
 
   auto wireBuf = std::move(builtOut.second);
@@ -1086,7 +1089,7 @@ TEST_F(QuicWriteCodecTest, NoSpaceConnClose) {
   setupCommonExpects(pktBuilder);
   std::string reasonPhrase("You are all fired");
   ConnectionCloseFrame connCloseFrame(
-      TransportErrorCode::PROTOCOL_VIOLATION, reasonPhrase);
+      QuicErrorCode(TransportErrorCode::PROTOCOL_VIOLATION), reasonPhrase);
   EXPECT_EQ(0, writeFrame(std::move(connCloseFrame), pktBuilder));
 }
 
@@ -1095,16 +1098,18 @@ TEST_F(QuicWriteCodecTest, DecodeAppCloseLarge) {
   setupCommonExpects(pktBuilder);
   std::string reasonPhrase;
   reasonPhrase.resize(kMaxReasonPhraseLength + 10);
-  ApplicationCloseFrame applicationCloseFrame(
+  ConnectionCloseFrame applicationCloseFrame(
-      GenericApplicationErrorCode::UNKNOWN, reasonPhrase);
+      QuicErrorCode(GenericApplicationErrorCode::UNKNOWN),
+      reasonPhrase,
+      quic::FrameType::CONNECTION_CLOSE_APP_ERR);
   writeFrame(std::move(applicationCloseFrame), pktBuilder);
 
   auto builtOut = std::move(pktBuilder).buildPacket();
   auto regularPacket = builtOut.first;
-  auto& resultAppCloseFrame =
+  auto& resultAppCloseFrame = *regularPacket.frames[0].asConnectionCloseFrame();
-      *regularPacket.frames[0].asApplicationCloseFrame();
   EXPECT_EQ(
-      GenericApplicationErrorCode::UNKNOWN, resultAppCloseFrame.errorCode);
+      quic::FrameType::CONNECTION_CLOSE_APP_ERR,
+      resultAppCloseFrame.closingFrameType);
   EXPECT_EQ(resultAppCloseFrame.reasonPhrase, reasonPhrase);
 
   auto wireBuf = std::move(builtOut.second);

quic/logging/BaseQLogger.cpp

@@ -111,12 +111,6 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
             frame.errorCode, frame.reasonPhrase, frame.closingFrameType));
         break;
       }
-      case QuicFrame::Type::ApplicationCloseFrame_E: {
-        const auto& frame = *quicFrame.asApplicationCloseFrame();
-        event->frames.push_back(std::make_unique<ApplicationCloseFrameLog>(
-            frame.errorCode, frame.reasonPhrase));
-        break;
-      }
       case QuicFrame::Type::MaxDataFrame_E: {
         const auto& frame = *quicFrame.asMaxDataFrame();
         event->frames.push_back(
@@ -221,13 +215,6 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
             frame.errorCode, frame.reasonPhrase, frame.closingFrameType));
         break;
       }
-      case QuicWriteFrame::Type::ApplicationCloseFrame_E: {
-        const ApplicationCloseFrame& frame =
-            *quicFrame.asApplicationCloseFrame();
-        event->frames.push_back(std::make_unique<ApplicationCloseFrameLog>(
-            frame.errorCode, frame.reasonPhrase));
-        break;
-      }
       case QuicWriteFrame::Type::MaxDataFrame_E: {
         const MaxDataFrame& frame = *quicFrame.asMaxDataFrame();
         event->frames.push_back(

quic/logging/QLoggerTypes.cpp

@@ -6,6 +6,7 @@
  *
  */
 #include <quic/logging/QLoggerTypes.h>
+#include <quic/QuicException.h>
 #include <quic/logging/QLoggerConstants.h>
 
 namespace quic {
@@ -28,21 +29,23 @@ folly::dynamic RstStreamFrameLog::toDynamic() const {
 
 folly::dynamic ConnectionCloseFrameLog::toDynamic() const {
   folly::dynamic d = folly::dynamic::object();
-  d["frame_type"] = toString(FrameType::CONNECTION_CLOSE);
+
+  auto isTransportErrorCode = errorCode.asTransportErrorCode();
+  auto isApplicationErrorCode = errorCode.asApplicationErrorCode();
+  auto isLocalErrorCode = errorCode.asLocalErrorCode();
+
+  if (isTransportErrorCode || isLocalErrorCode) {
+    d["frame_type"] = toString(FrameType::CONNECTION_CLOSE);
+  } else if (isApplicationErrorCode) {
+    d["frame_type"] = toString(FrameType::CONNECTION_CLOSE_APP_ERR);
+  }
+
   d["error_code"] = toString(errorCode);
   d["reason_phrase"] = reasonPhrase;
   d["closing_frame_type"] = toString(closingFrameType);
   return d;
 }
 
-folly::dynamic ApplicationCloseFrameLog::toDynamic() const {
-  folly::dynamic d = folly::dynamic::object();
-  d["frame_type"] = toString(FrameType::APPLICATION_CLOSE);
-  d["error_code"] = errorCode;
-  d["reason_phrase"] = reasonPhrase;
-  return d;
-}
-
 folly::dynamic MaxDataFrameLog::toDynamic() const {
   folly::dynamic d = folly::dynamic::object();
   d["frame_type"] = toString(FrameType::MAX_DATA);

quic/logging/QLoggerTypes.h

@@ -51,15 +51,15 @@ class RstStreamFrameLog : public QLogFrame {
 
 class ConnectionCloseFrameLog : public QLogFrame {
  public:
-  TransportErrorCode errorCode;
+  QuicErrorCode errorCode;
   std::string reasonPhrase;
   FrameType closingFrameType;
 
   ConnectionCloseFrameLog(
-      TransportErrorCode errorCodeIn,
+      QuicErrorCode errorCodeIn,
       std::string reasonPhraseIn,
       FrameType closingFrameTypeIn)
-      : errorCode{errorCodeIn},
+      : errorCode{std::move(errorCodeIn)},
         reasonPhrase{std::move(reasonPhraseIn)},
         closingFrameType{closingFrameTypeIn} {}
 
@@ -67,20 +67,6 @@ class ConnectionCloseFrameLog : public QLogFrame {
   folly::dynamic toDynamic() const override;
 };
 
-class ApplicationCloseFrameLog : public QLogFrame {
- public:
-  ApplicationErrorCode errorCode;
-  std::string reasonPhrase;
-
-  ApplicationCloseFrameLog(
-      ApplicationErrorCode errorCodeIn,
-      std::string reasonPhraseIn)
-      : errorCode{errorCodeIn}, reasonPhrase{std::move(reasonPhraseIn)} {}
-
-  ~ApplicationCloseFrameLog() override = default;
-  folly::dynamic toDynamic() const override;
-};
-
 class MaxDataFrameLog : public QLogFrame {
  public:
   uint64_t maximumData;

quic/server/state/ServerStateMachine.cpp

@@ -951,25 +951,6 @@ void onServerReadDataFromOpen(
               "Peer closed", TransportErrorCode::NO_ERROR);
           break;
         }
-        case QuicFrame::Type::ApplicationCloseFrame_E: {
-          isNonProbingPacket = true;
-          ApplicationCloseFrame& appClose =
-              *quicFrame.asApplicationCloseFrame();
-          auto errMsg = folly::to<std::string>(
-              "Server closed by peer reason=", appClose.reasonPhrase);
-          VLOG(10) << errMsg << " " << conn;
-          // we want to deliver app callbacks with the peer supplied error,
-          // but send a NO_ERROR to the peer.
-          QUIC_TRACE(recvd_close, conn, errMsg.c_str());
-          if (conn.qLogger) {
-            conn.qLogger->addTransportStateUpdate(getPeerClose(errMsg));
-          }
-          conn.peerConnectionError = std::make_pair(
-              QuicErrorCode(appClose.errorCode), std::move(errMsg));
-          throw QuicTransportException(
-              "Peer closed", TransportErrorCode::NO_ERROR);
-          break;
-        }
         case QuicFrame::Type::PaddingFrame_E: {
           break;
         }
@@ -1140,7 +1121,6 @@ void onServerReadDataFromClosed(
   }
 
   auto& regularPacket = *regularOptional;
-  auto protectionLevel = regularPacket.header.getProtectionType();
   auto packetNum = regularPacket.header.getPacketSequenceNum();
   auto pnSpace = regularPacket.header.getPacketNumberSpace();
   if (conn.qLogger) {
@@ -1148,9 +1128,6 @@ void onServerReadDataFromClosed(
   }
   QUIC_TRACE(packet_recvd, conn, packetNum, packetSize);
 
-  bool isProtectedPacket = protectionLevel == ProtectionType::ZeroRtt ||
-      protectionLevel == ProtectionType::KeyPhaseZero ||
-      protectionLevel == ProtectionType::KeyPhaseOne;
 
   // Only process the close frames in the packet
   for (auto& quicFrame : regularPacket.frames) {
@@ -1170,25 +1147,6 @@ void onServerReadDataFromClosed(
             QuicErrorCode(connFrame.errorCode), std::move(errMsg));
         break;
       }
-      case QuicFrame::Type::ApplicationCloseFrame_E: {
-        if (!isProtectedPacket) {
-          return;
-        }
-        ApplicationCloseFrame& appClose = *quicFrame.asApplicationCloseFrame();
-        auto errMsg = folly::to<std::string>(
-            "Server closed by peer reason=", appClose.reasonPhrase);
-        VLOG(10) << errMsg << " " << conn;
-        if (conn.qLogger) {
-          conn.qLogger->addTransportStateUpdate(getPeerClose(errMsg));
-        }
-        // we want to deliver app callbacks with the peer supplied error,
-        // but send a NO_ERROR to the peer.
-        QUIC_TRACE(recvd_close, conn, errMsg.c_str());
-        conn.peerConnectionError = std::make_pair(
-            QuicErrorCode(appClose.errorCode), std::move(errMsg));
-
-        break;
-      }
       default:
         break;
     }

quic/server/test/QuicServerTransportTest.cpp

@@ -870,9 +870,7 @@ TEST_F(QuicServerTransportTest, IdleTimeoutExpired) {
   EXPECT_TRUE(server->isClosed());
   auto serverReadCodec = makeClientEncryptedCodec();
   EXPECT_FALSE(verifyFramePresent(
-      serverWrites,
+      serverWrites, *serverReadCodec, QuicFrame::Type::ConnectionCloseFrame_E));
-      *serverReadCodec,
-      QuicFrame::Type::ApplicationCloseFrame_E));
   EXPECT_FALSE(verifyFramePresent(
       serverWrites, *serverReadCodec, QuicFrame::Type::ConnectionCloseFrame_E));
 }
@@ -901,7 +899,7 @@ TEST_F(QuicServerTransportTest, TestCloseConnectionWithError) {
   EXPECT_TRUE(verifyFramePresent(
       serverWrites,
       *makeClientEncryptedCodec(),
-      QuicFrame::Type::ApplicationCloseFrame_E));
+      QuicFrame::Type::ConnectionCloseFrame_E));
 }
 
 TEST_F(QuicServerTransportTest, TestCloseConnectionWithNoError) {
@@ -911,7 +909,7 @@ TEST_F(QuicServerTransportTest, TestCloseConnectionWithNoError) {
   EXPECT_TRUE(verifyFramePresent(
       serverWrites,
       *makeClientEncryptedCodec(),
-      QuicFrame::Type::ApplicationCloseFrame_E));
+      QuicFrame::Type::ConnectionCloseFrame_E));
 }
 
 TEST_F(QuicServerTransportTest, TestClientAddressChanges) {
@@ -968,7 +966,7 @@ TEST_F(QuicServerTransportTest, TestCloseConnectionWithNoErrorPendingStreams) {
   EXPECT_TRUE(verifyFramePresent(
       serverWrites,
       *makeClientEncryptedCodec(),
-      QuicFrame::Type::ApplicationCloseFrame_E));
+      QuicFrame::Type::ConnectionCloseFrame_E));
 }
 
 TEST_F(QuicServerTransportTest, ReceivePacketAfterLocalError) {
@@ -1052,7 +1050,8 @@ TEST_F(QuicServerTransportTest, ReceiveCloseAfterLocalError) {
       std::move(header2),
       0 /* largestAcked */);
   std::string errMsg = "Mind the gap";
-  ConnectionCloseFrame connClose(TransportErrorCode::NO_ERROR, errMsg);
+  ConnectionCloseFrame connClose(
+      QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg);
   writeFrame(std::move(connClose), builder2);
 
   auto packet2 = std::move(builder2).buildPacket();
@@ -1101,7 +1100,8 @@ TEST_F(QuicServerTransportTest, NoDataExceptCloseProcessedAfterClosing) {
       true);
   writeStreamFrameData(builder, buf->clone(), buf->computeChainDataLength());
   std::string errMsg = "Mind the gap";
-  ConnectionCloseFrame connClose(TransportErrorCode::NO_ERROR, errMsg);
+  ConnectionCloseFrame connClose(
+      QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg);
   writeFrame(std::move(connClose), builder);
 
   auto packet = std::move(builder).buildPacket();
@@ -1112,7 +1112,7 @@ TEST_F(QuicServerTransportTest, NoDataExceptCloseProcessedAfterClosing) {
   EXPECT_TRUE(verifyFramePresent(
       serverWrites,
       *makeClientEncryptedCodec(),
-      QuicFrame::Type::ApplicationCloseFrame_E));
+      QuicFrame::Type::ConnectionCloseFrame_E));
   EXPECT_TRUE(hasNotReceivedNewPacketsSinceLastCloseSent(server->getConn()));
   serverWrites.clear();
 
@@ -1129,7 +1129,7 @@ TEST_F(QuicServerTransportTest, NoDataExceptCloseProcessedAfterClosing) {
   EXPECT_FALSE(verifyFramePresent(
       serverWrites,
       *makeClientEncryptedCodec(),
-      QuicFrame::Type::ApplicationCloseFrame_E));
+      QuicFrame::Type::ConnectionCloseFrame_E));
   EXPECT_EQ(server->getConn().streamManager->streamCount(), 0);
 }
 
@@ -1842,7 +1842,8 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionClose) {
       0 /* largestAcked */,
       QuicVersion::MVFST);
   std::string errMsg = "Stand clear of the closing doors, please";
-  ConnectionCloseFrame connClose(TransportErrorCode::NO_ERROR, errMsg);
+  ConnectionCloseFrame connClose(
+      QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg);
   writeFrame(std::move(connClose), builder);
   auto packet = std::move(builder).buildPacket();
   EXPECT_CALL(connCallback, onConnectionEnd());
@@ -1878,8 +1879,10 @@ TEST_F(QuicServerTransportTest, ReceiveApplicationClose) {
       std::move(header),
       0 /* largestAcked */,
       QuicVersion::MVFST);
+
   std::string errMsg = "Stand clear of the closing doors, please";
-  ApplicationCloseFrame appClose(GenericApplicationErrorCode::UNKNOWN, errMsg);
+  ConnectionCloseFrame appClose(
+      QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), errMsg);
   writeFrame(std::move(appClose), builder);
   auto packet = std::move(builder).buildPacket();
 
@@ -1918,7 +1921,8 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionCloseTwice) {
       0 /* largestAcked */,
       QuicVersion::MVFST);
   std::string errMsg = "Mind the gap";
-  ConnectionCloseFrame connClose(TransportErrorCode::NO_ERROR, errMsg);
+  ConnectionCloseFrame connClose(
+      QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg);
   writeFrame(std::move(connClose), builder);
   auto packet = std::move(builder).buildPacket();
   EXPECT_CALL(connCallback, onConnectionEnd());