diff --git a/quic/QuicException.cpp b/quic/QuicException.cpp index 9e90b76bd..fbc7dce64 100644 --- a/quic/QuicException.cpp +++ b/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(*code.asApplicationErrorCode()); case QuicErrorCode::Type::LocalErrorCode_E: return toString(*code.asLocalErrorCode()).str(); diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 597c37f66..6a80b868b 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/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 diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 654006576..ccee390b7 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -1243,7 +1243,9 @@ TEST_F(QuicTransportImplTest, RegisterDeliveryCallbackLowerThanExpectedClose) { TEST_F(QuicTransportImplTest, TestNotifyPendingConnWriteOnCloseWithoutError) { NiceMock 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 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 wcbConn; NiceMock rcb; NiceMock 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); diff --git a/quic/client/test/QuicClientTransportTest.cpp b/quic/client/test/QuicClientTransportTest.cpp index 7fbc97001..eaf8983b9 100644 --- a/quic/client/test/QuicClientTransportTest.cpp +++ b/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(tmp.get()); - EXPECT_EQ(event->error, kNoError); - auto reason = folly::to( - "Server: ", kNoError, ", Peer: isReset: ", 0, ", Peer: isAbandon: ", 0); - EXPECT_EQ(event->reason, reason); + EXPECT_EQ(event->error, "No Error"); + EXPECT_EQ(event->reason, "No Error"); EXPECT_TRUE(event->drainConnection); EXPECT_TRUE(event->sendCloseImmediately); } diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index f636f5425..bd1961b8f 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/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) {