diff --git a/quic/api/QuicSocket.h b/quic/api/QuicSocket.h index 30b97eb72..c3960ccbd 100644 --- a/quic/api/QuicSocket.h +++ b/quic/api/QuicSocket.h @@ -798,15 +798,9 @@ class QuicSocket { * transport when the peer has acknowledged the receipt of all the data/eof * passed to write. * - * A returned IOBuf indicates that the passed data exceeded the transport - * flow control window or send buffer space. The application must call write - * with this data again later, and should be a signal to apply backpressure. - * If EOF was true or a delivery callback was set they also need to be - * passed again later. See notifyPendingWrite to register for a callback. - * * An error code is present if there was an error with the write. */ - using WriteResult = folly::Expected; + using WriteResult = folly::Expected; virtual WriteResult writeChain( StreamId id, Buf data, diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 3e8ef7aab..5865e03af 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -1899,7 +1899,7 @@ QuicSocket::WriteResult QuicTransportBase::writeChain( std::string("writeChain() error"))); return folly::makeUnexpected(LocalErrorCode::INTERNAL_ERROR); } - return nullptr; + return folly::unit; } folly::Expected diff --git a/quic/api/test/MockQuicSocket.h b/quic/api/test/MockQuicSocket.h index 4a5e8bba9..95749df0f 100644 --- a/quic/api/test/MockQuicSocket.h +++ b/quic/api/test/MockQuicSocket.h @@ -140,21 +140,15 @@ class MockQuicSocket : public QuicSocket { MOCK_METHOD1( unregisterStreamWriteCallback, folly::Expected(StreamId)); - folly::Expected writeChain( + folly::Expected writeChain( StreamId id, Buf data, bool eof, bool cork, DeliveryCallback* cb) override { SharedBuf sharedData(data.release()); - auto res = writeChain(id, sharedData, eof, cork, cb); - if (res.hasError()) { - return folly::makeUnexpected(res.error()); - } else { - return Buf(res.value()); - } + return writeChain(id, sharedData, eof, cork, cb); } - using WriteResult = folly::Expected; MOCK_METHOD5( writeChain, WriteResult(StreamId, SharedBuf, bool, bool, DeliveryCallback*)); diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 8976044fe..f7812ad96 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -1043,7 +1043,7 @@ TEST_F(QuicTransportImplTest, ConnectionErrorOnWrite) { auto stream = transport->createBidirectionalStream().value(); EXPECT_CALL(*socketPtr, write(_, _)) .WillOnce(SetErrnoAndReturn(ENETUNREACH, -1)); - QuicSocket::WriteResult result = transport->writeChain( + transport->writeChain( stream, folly::IOBuf::copyBuffer("Hey"), true, false, nullptr); transport->addDataToStream( stream, StreamBuffer(folly::IOBuf::copyBuffer("Data"), 0)); @@ -1074,7 +1074,7 @@ TEST_F(QuicTransportImplTest, ReadErrorUnsanitizedErrorMsg) { throw std::runtime_error("You need to calm down."); return 0; })); - QuicSocket::WriteResult result = transport->writeChain( + transport->writeChain( stream, folly::IOBuf::copyBuffer("You are being too loud."), true, @@ -1097,7 +1097,7 @@ TEST_F(QuicTransportImplTest, ConnectionErrorUnhandledException) { throw std::runtime_error("Well there's your problem"); return 0; })); - QuicSocket::WriteResult result = transport->writeChain( + transport->writeChain( stream, folly::IOBuf::copyBuffer("Hey"), true, false, nullptr); transport->addDataToStream( stream, StreamBuffer(folly::IOBuf::copyBuffer("Data"), 0)); diff --git a/quic/samples/echo/EchoClient.h b/quic/samples/echo/EchoClient.h index fffab757f..82bf5a43c 100644 --- a/quic/samples/echo/EchoClient.h +++ b/quic/samples/echo/EchoClient.h @@ -169,11 +169,6 @@ class EchoClient : public quic::QuicSocket::ConnectionCallback, auto res = quicClient_->writeChain(id, message->clone(), true, false); if (res.hasError()) { LOG(ERROR) << "EchoClient writeChain error=" << uint32_t(res.error()); - } else if (res.value()) { - LOG(INFO) << "EchoClient socket did not accept all data, buffering len=" - << res.value()->computeChainDataLength(); - data.append(std::move(res.value())); - quicClient_->notifyPendingWriteOnStream(id, this); } else { auto str = message->moveToFbString().toStdString(); LOG(INFO) << "EchoClient wrote \"" << str << "\"" diff --git a/quic/samples/echo/EchoHandler.h b/quic/samples/echo/EchoHandler.h index 01aacde6b..cecc8154a 100644 --- a/quic/samples/echo/EchoHandler.h +++ b/quic/samples/echo/EchoHandler.h @@ -105,11 +105,6 @@ class EchoHandler : public quic::QuicSocket::ConnectionCallback, sock->writeChain(id, std::move(echoedData), true, false, nullptr); if (res.hasError()) { LOG(ERROR) << "write error=" << toString(res.error()); - } else if (res.value()) { - LOG(INFO) << "socket did not accept all data, buffering len=" - << res.value()->computeChainDataLength(); - data.first.append(std::move(res.value())); - sock->notifyPendingWriteOnStream(id, this); } else { // echo is done, clear EOF data.second = false; @@ -149,11 +144,6 @@ class EchoHandler : public quic::QuicSocket::ConnectionCallback, auto res = sock->writeChain(id, originalData.move(), true, false, nullptr); if (res.hasError()) { LOG(ERROR) << "write error=" << toString(res.error()); - } else if (res.value()) { - LOG(INFO) << "socket did not accept all data, buffering len=" - << res.value()->computeChainDataLength(); - originalData.append(std::move(res.value())); - sock->notifyPendingWriteOnStream(id, this); } else { // echo is done, clear EOF data.second = false; diff --git a/quic/server/test/QuicSocketTest.cpp b/quic/server/test/QuicSocketTest.cpp index 185d822e1..306f218b3 100644 --- a/quic/server/test/QuicSocketTest.cpp +++ b/quic/server/test/QuicSocketTest.cpp @@ -49,7 +49,7 @@ TEST_F(QuicSocketTest, simple) { EXPECT_CALL(*socket_, readNaked(3, _)) .WillOnce(Return(readResult("hello world", true))); EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr)) - .WillOnce(Return(nullptr)); + .WillOnce(Return(folly::unit)); handler_.readAvailable(3); } @@ -64,29 +64,6 @@ TEST_F(QuicSocketTest, multiple_reads) { EXPECT_CALL(*socket_, readNaked(3, _)) .WillOnce(Return(readResult("world", true))); EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr)) - .WillOnce(Return(nullptr)); + .WillOnce(Return(folly::unit)); handler_.readAvailable(3); } - -TEST_F(QuicSocketTest, blocked_write) { - InSequence enforceOrder; - openStream(3); - - EXPECT_CALL(*socket_, readNaked(3, _)) - .WillOnce(Return(readResult("hello world", true))); - EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr)) - .WillOnce(Invoke( - [](StreamId, - MockQuicSocket::SharedBuf b, - bool, - bool, - MockQuicSocket::DeliveryCallback*) { - return IOBuf::copyBuffer(b->data(), b->length()).release(); - })); - EXPECT_CALL(*socket_, notifyPendingWriteOnStream(3, _)); - handler_.readAvailable(3); - - EXPECT_CALL(*socket_, writeChain(3, _, true, false, nullptr)) - .WillOnce(Return(nullptr)); - handler_.onStreamWriteReady(3, 100); -}