Change onReadData() to return an error

Summary:
All according to plan: https://fburl.com/gdoc/pebccgi1
Changing function definitions to return errors while still throwing.

Reviewed By: sharmafb

Differential Revision: D69567329

fbshipit-source-id: 5d40ee32fe185d5674785632a9a13e4cef996988

quic/api/QuicTransportBaseLite.cpp

@@ -126,7 +126,12 @@ void QuicTransportBaseLite::onNetworkData(
 
     auto packets = std::move(networkData).movePackets();
     for (auto& packet : packets) {
-      onReadData(peer, std::move(packet));
+      auto res = onReadData(peer, std::move(packet));
+      if (res.hasError()) {
+        VLOG(4) << __func__ << " " << res.error().message << " " << *this;
+        exceptionCloseWhat_ = res.error().message;
+        return closeImpl(res.error());
+      }
       if (conn_->peerConnectionError) {
         closeImpl(QuicError(
             QuicErrorCode(TransportErrorCode::NO_ERROR), "Peer closed"));

quic/api/QuicTransportBaseLite.h

@@ -43,7 +43,7 @@ class QuicTransportBaseLite : virtual public QuicSocketLite,
    * The sub-class may throw an exception if there was an error in processing
    * the packet in which case the connection will be closed.
    */
-  virtual void onReadData(
+  virtual folly::Expected<folly::Unit, QuicError> onReadData(
       const folly::SocketAddress& peer,
       ReceivedUdpPacket&& udpPacket) = 0;
 

quic/api/test/QuicTransportBaseTest.cpp

@@ -283,10 +283,11 @@ class TestQuicTransport
     return lossTimeout_.getTimerCallbackTimeRemaining();
   }
 
-  void onReadData(const folly::SocketAddress&, ReceivedUdpPacket&& udpPacket)
-      override {
+  folly::Expected<folly::Unit, QuicError> onReadData(
+      const folly::SocketAddress&,
+      ReceivedUdpPacket&& udpPacket) override {
     if (udpPacket.buf.empty()) {
-      return;
+      return folly::unit;
     }
     folly::io::Cursor cursor(udpPacket.buf.front());
     while (!cursor.isAtEnd()) {
@@ -332,6 +333,7 @@ class TestQuicTransport
         conn_->streamManager->updatePeekableStreams(*stream);
       }
     }
+    return folly::unit;
   }
 
   void writeData() override {

quic/api/test/TestQuicTransport.h

@@ -85,9 +85,11 @@ class TestQuicTransport
     return writeLooper_->isPacingScheduled();
   }
 
-  void onReadData(
+  folly::Expected<folly::Unit, QuicError> onReadData(
       const folly::SocketAddress& /* peer */,
-      ReceivedUdpPacket&& /* udpPacket */) noexcept override {}
+      ReceivedUdpPacket&& /* udpPacket */) noexcept override {
+    return folly::unit;
+  }
 
   void writeData() override {
     if (closed) {

quic/client/QuicClientTransportLite.cpp

@@ -835,7 +835,7 @@ void QuicClientTransportLite::processUdpPacketData(
   }
 }
 
-void QuicClientTransportLite::onReadData(
+folly::Expected<folly::Unit, QuicError> QuicClientTransportLite::onReadData(
     const folly::SocketAddress& peer,
     ReceivedUdpPacket&& udpPacket) {
   if (closeState_ == CloseState::CLOSED) {
@@ -845,7 +845,7 @@ void QuicClientTransportLite::onReadData(
     if (conn_->qLogger) {
       conn_->qLogger->addPacketDrop(0, kAlreadyClosed);
     }
-    return;
+    return folly::unit;
   }
   bool waitingForFirstPacket = !hasReceivedUdpPackets(*conn_);
   processUdpPacket(peer, std::move(udpPacket));
@@ -873,6 +873,7 @@ void QuicClientTransportLite::onReadData(
   }
 
   maybeSendTransportKnobs();
+  return folly::unit;
 }
 
 QuicSocketLite::WriteResult QuicClientTransportLite::writeBufMeta(

quic/client/QuicClientTransportLite.h

@@ -156,7 +156,7 @@ class QuicClientTransportLite
   bool hasZeroRttWriteCipher() const;
 
   // From QuicTransportBase
-  void onReadData(
+  folly::Expected<folly::Unit, QuicError> onReadData(
       const folly::SocketAddress& peer,
       ReceivedUdpPacket&& udpPacket) override;
   void writeData() override;

quic/server/QuicServerTransport.cpp

@@ -156,7 +156,7 @@ void QuicServerTransport::setServerConnectionIdRejector(
   }
 }
 
-void QuicServerTransport::onReadData(
+folly::Expected<folly::Unit, QuicError> QuicServerTransport::onReadData(
     const folly::SocketAddress& peer,
     ReceivedUdpPacket&& udpPacket) {
   ServerEvents::ReadData readData;
@@ -170,7 +170,7 @@ void QuicServerTransport::onReadData(
   processPendingData(true);
 
   if (closeState_ == CloseState::CLOSED) {
-    return;
+    return folly::unit;
   }
   if (!notifiedRouting_ && routingCb_ && conn_->serverConnectionId) {
     notifiedRouting_ = true;
@@ -202,6 +202,8 @@ void QuicServerTransport::onReadData(
   maybeIssueConnectionIds();
   maybeNotifyTransportReady();
   maybeUpdateCongestionControllerFromTicket();
+
+  return folly::unit;
 }
 
 void QuicServerTransport::accept() {

quic/server/QuicServerTransport.h

@@ -137,7 +137,7 @@ class QuicServerTransport
   void verifiedClientAddress();
 
   // From QuicTransportBase
-  void onReadData(
+  folly::Expected<folly::Unit, QuicError> onReadData(
       const folly::SocketAddress& peer,
       ReceivedUdpPacket&& udpPacket) override;
   void writeData() override;