Fire readError and remove readCb only when reliable data has been read

Summary: See title

Reviewed By: afrind

Differential Revision: D67766485

fbshipit-source-id: 7283b438317f7b750e274414790777c7dd1572e9

quic/api/QuicTransportBaseLite.cpp

@@ -2456,7 +2456,11 @@ void QuicTransportBaseLite::invokeReadDataAndCallbacks(
     }
     auto readCb = callback->second.readCb;
     auto stream = CHECK_NOTNULL(conn_->streamManager->getStream(streamId));
-    if (readCb && stream->streamReadError) {
+    if (readCb && stream->streamReadError &&
+        (!stream->reliableSizeFromPeer ||
+         *stream->reliableSizeFromPeer <= stream->currentReadOffset)) {
+      // If we got a reliable reset from the peer, we don't fire the readError
+      // callback and remove it until we've read all of the reliable data.
       if (self->conn_->transportSettings
               .unidirectionalStreamsReadCallbacksFirst &&
           isUnidirectionalStream(streamId)) {

quic/api/test/QuicTransportBaseTest.cpp

@@ -975,6 +975,39 @@ TEST_P(QuicTransportImplTestBase, ReadCallbackDataAvailable) {
   transport.reset();
 }
 
+TEST_P(QuicTransportImplTestBase, ReliableResetReadCallback) {
+  auto stream = transport->createBidirectionalStream().value();
+  NiceMock<MockReadCallback> readCb;
+
+  transport->setReadCallback(stream, &readCb);
+  transport->addDataToStream(
+      stream,
+      StreamBuffer(
+          folly::IOBuf::copyBuffer("this string has 29 characters"), 0));
+  EXPECT_CALL(readCb, readAvailable(stream));
+  transport->driveReadCallbacks();
+
+  // Simulate receiving a reliable reset with a reliableSize of 29
+  receiveRstStreamSMHandler(
+      *transport->getStream(stream),
+      RstStreamFrame(stream, GenericApplicationErrorCode::UNKNOWN, 100, 29));
+
+  // The application hasn't yet read all of the reliable data, so we
+  // shouldn't fire the readError callback yet.
+  EXPECT_CALL(readCb, readAvailable(stream));
+  transport->driveReadCallbacks();
+
+  transport->read(stream, 29);
+
+  // The application has yet read all of the reliable data, so we should fire
+  // the readError callback.
+  EXPECT_CALL(
+      readCb, readError(stream, IsError(GenericApplicationErrorCode::UNKNOWN)));
+  transport->driveReadCallbacks();
+
+  transport.reset();
+}
+
 TEST_P(
     QuicTransportImplTestBase,
     ReadCallbackDataAvailableWithUnidirPrioritized) {