1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-01 01:44:22 +03:00

Remove remaining exception throw from QuicPacketScheduler

Summary: Continuing the theme, mostly involved getting rid of it from the stream writes and connecting two sides of Expecteds.

Reviewed By: kvtsoy

Differential Revision: D74585096

fbshipit-source-id: d81e6cbbc09981d0f1519494cf87e93e93e892db
This commit is contained in:
Matt Joras
2025-05-12 13:34:12 -07:00
committed by Facebook GitHub Bot
parent 40e6c833fa
commit 5bf490e4b8
3 changed files with 125 additions and 72 deletions

View File

@ -319,7 +319,10 @@ FrameScheduler::scheduleFramesForPacket(
pingFrameScheduler_->writePing(wrapper); pingFrameScheduler_->writePing(wrapper);
} }
if (streamFrameScheduler_ && streamFrameScheduler_->hasPendingData()) { if (streamFrameScheduler_ && streamFrameScheduler_->hasPendingData()) {
streamFrameScheduler_->writeStreams(wrapper); auto result = streamFrameScheduler_->writeStreams(wrapper);
if (result.hasError()) {
return folly::makeUnexpected(result.error());
}
} }
if (datagramFrameScheduler_ && if (datagramFrameScheduler_ &&
datagramFrameScheduler_->hasPendingDatagramFrames()) { datagramFrameScheduler_->hasPendingDatagramFrames()) {
@ -386,7 +389,7 @@ folly::StringPiece FrameScheduler::name() const {
return name_; return name_;
} }
bool StreamFrameScheduler::writeStreamLossBuffers( folly::Expected<bool, QuicError> StreamFrameScheduler::writeStreamLossBuffers(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
QuicStreamState& stream) { QuicStreamState& stream) {
bool wroteStreamFrame = false; bool wroteStreamFrame = false;
@ -404,8 +407,7 @@ bool StreamFrameScheduler::writeStreamLossBuffers(
std::nullopt /* skipLenHint */, std::nullopt /* skipLenHint */,
stream.groupId); stream.groupId);
if (res.hasError()) { if (res.hasError()) {
throw QuicInternalException( return folly::makeUnexpected(res.error());
res.error().message, *res.error().code.asLocalErrorCode());
} }
auto dataLen = *res; auto dataLen = *res;
if (dataLen) { if (dataLen) {
@ -426,19 +428,28 @@ bool StreamFrameScheduler::writeStreamLossBuffers(
StreamFrameScheduler::StreamFrameScheduler(QuicConnectionStateBase& conn) StreamFrameScheduler::StreamFrameScheduler(QuicConnectionStateBase& conn)
: conn_(conn) {} : conn_(conn) {}
StreamFrameScheduler::StreamWriteResult StreamFrameScheduler::writeSingleStream( folly::Expected<StreamFrameScheduler::StreamWriteResult, QuicError>
StreamFrameScheduler::writeSingleStream(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
QuicStreamState& stream, QuicStreamState& stream,
uint64_t& connWritableBytes) { uint64_t& connWritableBytes) {
StreamWriteResult result = StreamWriteResult::NOT_LIMITED; StreamWriteResult result = StreamWriteResult::NOT_LIMITED;
if (!stream.lossBuffer.empty()) { if (!stream.lossBuffer.empty()) {
if (!writeStreamLossBuffers(builder, stream)) { auto writeResult = writeStreamLossBuffers(builder, stream);
if (writeResult.hasError()) {
return folly::makeUnexpected(writeResult.error());
}
if (!writeResult.value()) {
return StreamWriteResult::PACKET_FULL; return StreamWriteResult::PACKET_FULL;
} }
} }
if (stream.hasWritableData(true)) { if (stream.hasWritableData(true)) {
if (connWritableBytes > 0 || stream.hasWritableData(false)) { if (connWritableBytes > 0 || stream.hasWritableData(false)) {
if (!writeStreamFrame(builder, stream, connWritableBytes)) { auto writeResult = writeStreamFrame(builder, stream, connWritableBytes);
if (writeResult.hasError()) {
return folly::makeUnexpected(writeResult.error());
}
if (!writeResult.value()) {
return StreamWriteResult::PACKET_FULL; return StreamWriteResult::PACKET_FULL;
} }
result = (connWritableBytes == 0) ? StreamWriteResult::CONN_FC_LIMITED result = (connWritableBytes == 0) ? StreamWriteResult::CONN_FC_LIMITED
@ -450,7 +461,7 @@ StreamFrameScheduler::StreamWriteResult StreamFrameScheduler::writeSingleStream(
return result; return result;
} }
StreamId StreamFrameScheduler::writeStreamsHelper( folly::Expected<StreamId, QuicError> StreamFrameScheduler::writeStreamsHelper(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
const std::set<StreamId>& writableStreams, const std::set<StreamId>& writableStreams,
StreamId nextScheduledStream, StreamId nextScheduledStream,
@ -466,7 +477,10 @@ StreamId StreamFrameScheduler::writeStreamsHelper(
auto stream = conn_.streamManager->findStream(*writableStreamItr); auto stream = conn_.streamManager->findStream(*writableStreamItr);
CHECK(stream); CHECK(stream);
auto writeResult = writeSingleStream(builder, *stream, connWritableBytes); auto writeResult = writeSingleStream(builder, *stream, connWritableBytes);
if (writeResult == StreamWriteResult::PACKET_FULL) { if (writeResult.hasError()) {
return folly::makeUnexpected(writeResult.error());
}
if (writeResult.value() == StreamWriteResult::PACKET_FULL) {
break; break;
} }
writableStreamItr++; writableStreamItr++;
@ -477,7 +491,8 @@ StreamId StreamFrameScheduler::writeStreamsHelper(
return *writableStreamItr; return *writableStreamItr;
} }
void StreamFrameScheduler::writeStreamsHelper( folly::Expected<folly::Unit, QuicError>
StreamFrameScheduler::writeStreamsHelper(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
deprecated::PriorityQueue& writableStreams, deprecated::PriorityQueue& writableStreams,
uint64_t& connWritableBytes, uint64_t& connWritableBytes,
@ -498,12 +513,15 @@ void StreamFrameScheduler::writeStreamsHelper(
auto stream = CHECK_NOTNULL(conn_.streamManager->findStream(streamId)); auto stream = CHECK_NOTNULL(conn_.streamManager->findStream(streamId));
if (!stream->hasSchedulableData() && stream->hasSchedulableDsr()) { if (!stream->hasSchedulableData() && stream->hasSchedulableDsr()) {
// We hit a DSR stream // We hit a DSR stream
return; return folly::unit;
} }
CHECK(stream) << "streamId=" << streamId CHECK(stream) << "streamId=" << streamId
<< "inc=" << uint64_t(level.incremental); << "inc=" << uint64_t(level.incremental);
if (writeSingleStream(builder, *stream, connWritableBytes) == auto writeResult = writeSingleStream(builder, *stream, connWritableBytes);
StreamWriteResult::PACKET_FULL) { if (writeResult.hasError()) {
return folly::makeUnexpected(writeResult.error());
}
if (writeResult.value() == StreamWriteResult::PACKET_FULL) {
break; break;
} }
auto remainingSpaceAfter = builder.remainingSpaceInPkt(); auto remainingSpaceAfter = builder.remainingSpaceInPkt();
@ -513,13 +531,15 @@ void StreamFrameScheduler::writeStreamsHelper(
bool forceNext = remainingSpaceAfter > 0; bool forceNext = remainingSpaceAfter > 0;
level.iterator->next(forceNext); level.iterator->next(forceNext);
if (streamPerPacket) { if (streamPerPacket) {
return; return folly::unit;
} }
} while (!level.iterator->end()); } while (!level.iterator->end());
} }
return folly::unit;
} }
void StreamFrameScheduler::writeStreamsHelper( folly::Expected<folly::Unit, QuicError>
StreamFrameScheduler::writeStreamsHelper(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
PriorityQueue& writableStreams, PriorityQueue& writableStreams,
uint64_t& connWritableBytes, uint64_t& connWritableBytes,
@ -538,13 +558,16 @@ void StreamFrameScheduler::writeStreamsHelper(
auto stream = CHECK_NOTNULL(conn_.streamManager->findStream(streamId)); auto stream = CHECK_NOTNULL(conn_.streamManager->findStream(streamId));
if (!stream->hasSchedulableData() && stream->hasSchedulableDsr()) { if (!stream->hasSchedulableData() && stream->hasSchedulableDsr()) {
// We hit a DSR stream // We hit a DSR stream
return; return folly::unit;
} }
CHECK(stream) << "streamId=" << streamId; CHECK(stream) << "streamId=" << streamId;
// TODO: this is counting STREAM frame overhead against the stream itself // TODO: this is counting STREAM frame overhead against the stream itself
auto lastWriteBytes = builder.remainingSpaceInPkt(); auto lastWriteBytes = builder.remainingSpaceInPkt();
auto writeResult = writeSingleStream(builder, *stream, connWritableBytes); auto writeResult = writeSingleStream(builder, *stream, connWritableBytes);
if (writeResult == StreamWriteResult::PACKET_FULL) { if (writeResult.hasError()) {
return folly::makeUnexpected(writeResult.error());
}
if (writeResult.value() == StreamWriteResult::PACKET_FULL) {
break; break;
} }
auto remainingSpaceAfter = builder.remainingSpaceInPkt(); auto remainingSpaceAfter = builder.remainingSpaceInPkt();
@ -554,7 +577,7 @@ void StreamFrameScheduler::writeStreamsHelper(
// we should erase the stream from writableStreams, the caller can rollback // we should erase the stream from writableStreams, the caller can rollback
// the transaction if the packet write fails // the transaction if the packet write fails
if (remainingSpaceAfter > 0) { if (remainingSpaceAfter > 0) {
if (writeResult == StreamWriteResult::CONN_FC_LIMITED) { if (writeResult.value() == StreamWriteResult::CONN_FC_LIMITED) {
conn_.streamManager->addConnFCBlockedStream(streamId); conn_.streamManager->addConnFCBlockedStream(streamId);
} }
writableStreams.erase(id); writableStreams.erase(id);
@ -562,44 +585,56 @@ void StreamFrameScheduler::writeStreamsHelper(
writableStreams.consume(lastWriteBytes); writableStreams.consume(lastWriteBytes);
} }
if (streamPerPacket) { if (streamPerPacket) {
return; return folly::unit;
} }
} }
return folly::unit;
} }
void StreamFrameScheduler::writeStreams(PacketBuilderInterface& builder) { folly::Expected<folly::Unit, QuicError> StreamFrameScheduler::writeStreams(
PacketBuilderInterface& builder) {
DCHECK(conn_.streamManager->hasWritable()); DCHECK(conn_.streamManager->hasWritable());
uint64_t connWritableBytes = getSendConnFlowControlBytesWire(conn_); uint64_t connWritableBytes = getSendConnFlowControlBytesWire(conn_);
// Write the control streams first as a naive binary priority mechanism. // Write the control streams first as a naive binary priority mechanism.
const auto& controlWriteQueue = conn_.streamManager->controlWriteQueue(); const auto& controlWriteQueue = conn_.streamManager->controlWriteQueue();
if (!controlWriteQueue.empty()) { if (!controlWriteQueue.empty()) {
conn_.schedulingState.nextScheduledControlStream = writeStreamsHelper( auto result = writeStreamsHelper(
builder, builder,
controlWriteQueue, controlWriteQueue,
conn_.schedulingState.nextScheduledControlStream, conn_.schedulingState.nextScheduledControlStream,
connWritableBytes, connWritableBytes,
conn_.transportSettings.streamFramePerPacket); conn_.transportSettings.streamFramePerPacket);
if (result.hasError()) {
return folly::makeUnexpected(result.error());
}
conn_.schedulingState.nextScheduledControlStream = result.value();
} }
auto* oldWriteQueue = conn_.streamManager->oldWriteQueue(); auto* oldWriteQueue = conn_.streamManager->oldWriteQueue();
QuicStreamState* nextStream{nullptr}; QuicStreamState* nextStream{nullptr};
if (oldWriteQueue) { if (oldWriteQueue) {
if (!oldWriteQueue->empty()) { if (!oldWriteQueue->empty()) {
writeStreamsHelper( auto result = writeStreamsHelper(
builder, builder,
*oldWriteQueue, *oldWriteQueue,
connWritableBytes, connWritableBytes,
conn_.transportSettings.streamFramePerPacket); conn_.transportSettings.streamFramePerPacket);
if (result.hasError()) {
return folly::makeUnexpected(result.error());
}
auto streamId = oldWriteQueue->getNextScheduledStream(); auto streamId = oldWriteQueue->getNextScheduledStream();
nextStream = conn_.streamManager->findStream(streamId); nextStream = conn_.streamManager->findStream(streamId);
} }
} else { } else {
auto& writeQueue = conn_.streamManager->writeQueue(); auto& writeQueue = conn_.streamManager->writeQueue();
if (!writeQueue.empty()) { if (!writeQueue.empty()) {
writeStreamsHelper( auto result = writeStreamsHelper(
builder, builder,
writeQueue, writeQueue,
connWritableBytes, connWritableBytes,
conn_.transportSettings.streamFramePerPacket); conn_.transportSettings.streamFramePerPacket);
if (result.hasError()) {
return folly::makeUnexpected(result.error());
}
if (!writeQueue.empty()) { if (!writeQueue.empty()) {
auto id = writeQueue.peekNextScheduledID(); auto id = writeQueue.peekNextScheduledID();
CHECK(id.isStreamID()); CHECK(id.isStreamID());
@ -615,6 +650,7 @@ void StreamFrameScheduler::writeStreams(PacketBuilderInterface& builder) {
if (nextStream && !nextStream->hasSchedulableData()) { if (nextStream && !nextStream->hasSchedulableData()) {
nextStreamDsr_ = true; nextStreamDsr_ = true;
} }
return folly::unit;
} }
bool StreamFrameScheduler::hasPendingData() const { bool StreamFrameScheduler::hasPendingData() const {
@ -624,7 +660,7 @@ bool StreamFrameScheduler::hasPendingData() const {
getSendConnFlowControlBytesWire(conn_) > 0)); getSendConnFlowControlBytesWire(conn_) > 0));
} }
bool StreamFrameScheduler::writeStreamFrame( folly::Expected<bool, QuicError> StreamFrameScheduler::writeStreamFrame(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
QuicStreamState& stream, QuicStreamState& stream,
uint64_t& connWritableBytes) { uint64_t& connWritableBytes) {
@ -653,8 +689,7 @@ bool StreamFrameScheduler::writeStreamFrame(
std::nullopt /* skipLenHint */, std::nullopt /* skipLenHint */,
stream.groupId); stream.groupId);
if (res.hasError()) { if (res.hasError()) {
throw QuicInternalException( return folly::makeUnexpected(res.error());
res.error().message, *res.error().code.asLocalErrorCode());
} }
auto dataLen = *res; auto dataLen = *res;
if (!dataLen) { if (!dataLen) {

View File

@ -77,13 +77,14 @@ class StreamFrameScheduler {
* Return: the first boolean indicates if at least one Blocked frame * Return: the first boolean indicates if at least one Blocked frame
* is written into the packet by writeStreams function. * is written into the packet by writeStreams function.
*/ */
void writeStreams(PacketBuilderInterface& builder); [[nodiscard]] folly::Expected<folly::Unit, QuicError> writeStreams(
PacketBuilderInterface& builder);
bool hasPendingData() const; bool hasPendingData() const;
private: private:
// Return true if this stream wrote some data // Return true if this stream wrote some data
bool writeStreamLossBuffers( [[nodiscard]] folly::Expected<bool, QuicError> writeStreamLossBuffers(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
QuicStreamState& stream); QuicStreamState& stream);
@ -100,25 +101,25 @@ class StreamFrameScheduler {
* flow control limited, or not limited by connection flow control. * flow control limited, or not limited by connection flow control.
*/ */
enum class StreamWriteResult { PACKET_FULL, NOT_LIMITED, CONN_FC_LIMITED }; enum class StreamWriteResult { PACKET_FULL, NOT_LIMITED, CONN_FC_LIMITED };
StreamWriteResult writeSingleStream( [[nodiscard]] folly::Expected<StreamWriteResult, QuicError> writeSingleStream(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
QuicStreamState& stream, QuicStreamState& stream,
uint64_t& connWritableBytes); uint64_t& connWritableBytes);
StreamId writeStreamsHelper( [[nodiscard]] folly::Expected<StreamId, QuicError> writeStreamsHelper(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
const std::set<StreamId>& writableStreams, const std::set<StreamId>& writableStreams,
StreamId nextScheduledStream, StreamId nextScheduledStream,
uint64_t& connWritableBytes, uint64_t& connWritableBytes,
bool streamPerPacket); bool streamPerPacket);
void writeStreamsHelper( [[nodiscard]] folly::Expected<folly::Unit, QuicError> writeStreamsHelper(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
deprecated::PriorityQueue& writableStreams, deprecated::PriorityQueue& writableStreams,
uint64_t& connWritableBytes, uint64_t& connWritableBytes,
bool streamPerPacket); bool streamPerPacket);
void writeStreamsHelper( [[nodiscard]] folly::Expected<folly::Unit, QuicError> writeStreamsHelper(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
PriorityQueue& writableStreams, PriorityQueue& writableStreams,
uint64_t& connWritableBytes, uint64_t& connWritableBytes,
@ -130,7 +131,7 @@ class StreamFrameScheduler {
* *
* Return: A boolean indicates if write is successful. * Return: A boolean indicates if write is successful.
*/ */
bool writeStreamFrame( [[nodiscard]] folly::Expected<bool, QuicError> writeStreamFrame(
PacketBuilderInterface& builder, PacketBuilderInterface& builder,
QuicStreamState& stream, QuicStreamState& stream,
uint64_t& connWritableBytes); uint64_t& connWritableBytes);

View File

@ -512,7 +512,9 @@ TEST_P(QuicPacketSchedulerTest, CryptoSchedulerOnlySingleLossFits) {
ChainedByteRangeHead(helloBuf), 0, false); ChainedByteRangeHead(helloBuf), 0, false);
conn.cryptoState->handshakeStream.lossBuffer.emplace_back( conn.cryptoState->handshakeStream.lossBuffer.emplace_back(
ChainedByteRangeHead(certBuf), 7, false); ChainedByteRangeHead(certBuf), 7, false);
EXPECT_TRUE(scheduler.writeCryptoData(builderWrapper)); auto result = scheduler.writeCryptoData(builderWrapper);
ASSERT_FALSE(result.hasError());
EXPECT_TRUE(result.value());
} }
TEST_P(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) { TEST_P(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) {
@ -1519,7 +1521,9 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
auto f3 = writeDataToStream(conn, stream3, "some data"); auto f3 = writeDataToStream(conn, stream3, "some data");
auto builder = setupMockPacketBuilder(); auto builder = setupMockPacketBuilder();
scheduler.writeStreams(*builder); auto result4 = scheduler.writeStreams(*builder);
ASSERT_FALSE(result4.hasError());
verifyStreamFrames(*builder, {f1, f2, f3}); verifyStreamFrames(*builder, {f1, f2, f3});
if (GetParam()) { if (GetParam()) {
EXPECT_TRUE(conn.streamManager->writeQueue().empty()); EXPECT_TRUE(conn.streamManager->writeQueue().empty());
@ -1544,11 +1548,13 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) {
// write a normal size packet from stream1 // write a normal size packet from stream1
auto builder = createPacketBuilder(conn); auto builder = createPacketBuilder(conn);
scheduler.writeStreams(builder); auto result = scheduler.writeStreams(builder);
ASSERT_FALSE(result.hasError());
// Should write frames for stream2, stream3, followed by stream1 again. // Should write frames for stream2, stream3, followed by stream1 again.
auto builder2 = setupMockPacketBuilder(); auto builder2 = setupMockPacketBuilder();
scheduler.writeStreams(*builder2); auto result2 = scheduler.writeStreams(*builder2);
ASSERT_FALSE(result2.hasError());
verifyStreamFrames(*builder2, {f2, f3, f1}); verifyStreamFrames(*builder2, {f2, f3, f1});
} }
@ -1571,16 +1577,17 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinNextsPer) {
// stream1 again. // stream1 again.
auto builder2 = auto builder2 =
setupMockPacketBuilder({1500, 0, 1400, 0, 1300, 1100, 1000, 0}); setupMockPacketBuilder({1500, 0, 1400, 0, 1300, 1100, 1000, 0});
scheduler.writeStreams(*builder2); auto result2 = scheduler.writeStreams(*builder2);
ASSERT_FALSE(result2.hasError());
builder2->advanceRemaining(); builder2->advanceRemaining();
ASSERT_EQ(nextScheduledStreamID(conn), stream1); ASSERT_EQ(nextScheduledStreamID(conn), stream1);
ASSERT_EQ(builder2->frames_.size(), 1); ASSERT_EQ(builder2->frames_.size(), 1);
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
ASSERT_EQ(builder2->frames_.size(), 2); ASSERT_EQ(builder2->frames_.size(), 2);
ASSERT_EQ(nextScheduledStreamID(conn), stream2); ASSERT_EQ(nextScheduledStreamID(conn), stream2);
builder2->advanceRemaining(); builder2->advanceRemaining();
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {stream1, stream1, stream2, stream3, stream1}); verifyStreamFrames(*builder2, {stream1, stream1, stream2, stream3, stream1});
} }
@ -1601,16 +1608,16 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinStreamPerPacket) {
// Write a normal size packet from stream1 // Write a normal size packet from stream1
auto builder = createPacketBuilder(conn); auto builder = createPacketBuilder(conn);
scheduler.writeStreams(builder); ASSERT_FALSE(scheduler.writeStreams(builder).hasError());
EXPECT_EQ(nextScheduledStreamID(conn), stream2); EXPECT_EQ(nextScheduledStreamID(conn), stream2);
// Should write frames for stream2, stream3, followed by stream1 again. // Should write frames for stream2, stream3, followed by stream1 again.
auto builder2 = setupMockPacketBuilder(); auto builder2 = setupMockPacketBuilder();
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f2}); verifyStreamFrames(*builder2, {f2});
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f3}); verifyStreamFrames(*builder2, {f3});
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f1}); verifyStreamFrames(*builder2, {f1});
} }
@ -1656,20 +1663,21 @@ TEST_P(
// Write a normal size packet from stream1 // Write a normal size packet from stream1
auto builder1 = createPacketBuilder(conn); auto builder1 = createPacketBuilder(conn);
scheduler.writeStreams(builder1); auto result = scheduler.writeStreams(builder1);
ASSERT_FALSE(result.hasError());
EXPECT_EQ(nextScheduledStreamID(conn), stream2); EXPECT_EQ(nextScheduledStreamID(conn), stream2);
// Should write frames for stream2, stream3, followed by an empty write. // Should write frames for stream2, stream3, followed by an empty write.
auto builder2 = setupMockPacketBuilder(); auto builder2 = setupMockPacketBuilder();
ASSERT_TRUE(scheduler.hasPendingData()); ASSERT_TRUE(scheduler.hasPendingData());
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
ASSERT_EQ(builder2->frames_.size(), 1); ASSERT_EQ(builder2->frames_.size(), 1);
ASSERT_TRUE(scheduler.hasPendingData()); ASSERT_TRUE(scheduler.hasPendingData());
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
ASSERT_EQ(builder2->frames_.size(), 2); ASSERT_EQ(builder2->frames_.size(), 2);
EXPECT_FALSE(scheduler.hasPendingData()); EXPECT_FALSE(scheduler.hasPendingData());
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f2, f3}); verifyStreamFrames(*builder2, {f2, f3});
} }
@ -1690,13 +1698,14 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerSequential) {
// Write a normal size packet from stream1 // Write a normal size packet from stream1
auto builder1 = createPacketBuilder(conn); auto builder1 = createPacketBuilder(conn);
scheduler.writeStreams(builder1); auto result = scheduler.writeStreams(builder1);
ASSERT_FALSE(result.hasError());
EXPECT_EQ(nextScheduledStreamID(conn), stream1); EXPECT_EQ(nextScheduledStreamID(conn), stream1);
// Should write frames for stream1, stream2, stream3, in that order. // Should write frames for stream1, stream2, stream3, in that order.
auto builder2 = setupMockPacketBuilder(); auto builder2 = setupMockPacketBuilder();
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f1, f2, f3}); verifyStreamFrames(*builder2, {f1, f2, f3});
} }
@ -1719,13 +1728,14 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerSequentialDefault) {
// Write a normal size packet from stream1 // Write a normal size packet from stream1
auto builder1 = createPacketBuilder(conn); auto builder1 = createPacketBuilder(conn);
scheduler.writeStreams(builder1); auto result = scheduler.writeStreams(builder1);
ASSERT_FALSE(result.hasError());
EXPECT_EQ(nextScheduledStreamID(conn), stream1); EXPECT_EQ(nextScheduledStreamID(conn), stream1);
// Should write frames for stream1, stream2, stream3, in that order. // Should write frames for stream1, stream2, stream3, in that order.
auto builder2 = setupMockPacketBuilder(); auto builder2 = setupMockPacketBuilder();
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f1, f2, f3}); verifyStreamFrames(*builder2, {f1, f2, f3});
} }
@ -1753,7 +1763,8 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) {
// This writes a normal size packet with 2, 4, 1 // This writes a normal size packet with 2, 4, 1
auto builder1 = createPacketBuilder(conn); auto builder1 = createPacketBuilder(conn);
scheduler.writeStreams(builder1); auto result = scheduler.writeStreams(builder1);
ASSERT_FALSE(result.hasError());
EXPECT_EQ(nextScheduledStreamID(conn), stream3); EXPECT_EQ(nextScheduledStreamID(conn), stream3);
EXPECT_EQ(conn.schedulingState.nextScheduledControlStream, stream2); EXPECT_EQ(conn.schedulingState.nextScheduledControlStream, stream2);
@ -1761,7 +1772,7 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) {
// 2 and 4 did not get removed from writable, so they get repeated here // 2 and 4 did not get removed from writable, so they get repeated here
// Should write frames for stream2, stream4, followed by stream 3 then 1. // Should write frames for stream2, stream4, followed by stream 3 then 1.
auto builder2 = setupMockPacketBuilder(); auto builder2 = setupMockPacketBuilder();
scheduler.writeStreams(*builder2); ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError());
verifyStreamFrames(*builder2, {f2, f4, f3, f1}); verifyStreamFrames(*builder2, {f2, f4, f3, f1});
@ -1782,7 +1793,7 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) {
writeDataToStream(conn, stream1, "some data"); writeDataToStream(conn, stream1, "some data");
auto builder1 = createPacketBuilder(conn); auto builder1 = createPacketBuilder(conn);
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
if (GetParam()) { if (GetParam()) {
EXPECT_TRUE(conn.streamManager->writeQueue().empty()); EXPECT_TRUE(conn.streamManager->writeQueue().empty());
@ -1803,7 +1814,9 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRemoveOne) {
auto f2 = writeDataToStream(conn, stream2, "some data"); auto f2 = writeDataToStream(conn, stream2, "some data");
auto builder = setupMockPacketBuilder(); auto builder = setupMockPacketBuilder();
scheduler.writeStreams(*builder); auto result4 = scheduler.writeStreams(*builder);
ASSERT_FALSE(result4.hasError());
verifyStreamFrames(*builder, {f1, f2}); verifyStreamFrames(*builder, {f1, f2});
// Manually remove a stream and set the next scheduled to that stream. // Manually remove a stream and set the next scheduled to that stream.
@ -1812,7 +1825,8 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRemoveOne) {
conn.streamManager->updateWritableStreams( conn.streamManager->updateWritableStreams(
*conn.streamManager->findStream(stream2)); *conn.streamManager->findStream(stream2));
scheduler.writeStreams(*builder); auto result = scheduler.writeStreams(*builder);
ASSERT_FALSE(result.hasError());
ASSERT_EQ(builder->frames_.size(), 1); ASSERT_EQ(builder->frames_.size(), 1);
verifyStreamFrames(*builder, {f2}); verifyStreamFrames(*builder, {f2});
} }
@ -1918,7 +1932,7 @@ TEST_P(QuicPacketSchedulerTest, HighPriNewDataBeforeLowPriLossData) {
conn, highPriStreamId, buildRandomInputData(conn.udpSendPacketLen * 10)); conn, highPriStreamId, buildRandomInputData(conn.udpSendPacketLen * 10));
auto builder1 = createPacketBuilder(conn); auto builder1 = createPacketBuilder(conn);
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
auto packet = std::move(builder1).buildPacket().packet; auto packet = std::move(builder1).buildPacket().packet;
EXPECT_EQ(1, packet.frames.size()); EXPECT_EQ(1, packet.frames.size());
@ -1952,7 +1966,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) {
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder1.encodePacketHeader().hasError()); ASSERT_FALSE(builder1.encodePacketHeader().hasError());
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
auto packet1 = std::move(builder1).buildPacket().packet; auto packet1 = std::move(builder1).buildPacket().packet;
ASSERT_FALSE( ASSERT_FALSE(
updateConnection( updateConnection(
@ -1982,7 +1996,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) {
std::move(shortHeader2), std::move(shortHeader2),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder2.encodePacketHeader().hasError()); ASSERT_FALSE(builder2.encodePacketHeader().hasError());
scheduler.writeStreams(builder2); ASSERT_FALSE(scheduler.writeStreams(builder2).hasError());
auto packet2 = std::move(builder2).buildPacket().packet; auto packet2 = std::move(builder2).buildPacket().packet;
ASSERT_FALSE( ASSERT_FALSE(
updateConnection( updateConnection(
@ -2030,7 +2044,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControlIgnoreDSR) {
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder1.encodePacketHeader().hasError()); ASSERT_FALSE(builder1.encodePacketHeader().hasError());
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
auto packet1 = std::move(builder1).buildPacket().packet; auto packet1 = std::move(builder1).buildPacket().packet;
ASSERT_FALSE( ASSERT_FALSE(
updateConnection( updateConnection(
@ -2075,7 +2089,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControlSequential) {
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder1.encodePacketHeader().hasError()); ASSERT_FALSE(builder1.encodePacketHeader().hasError());
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
auto packet1 = std::move(builder1).buildPacket().packet; auto packet1 = std::move(builder1).buildPacket().packet;
ASSERT_FALSE( ASSERT_FALSE(
updateConnection( updateConnection(
@ -2105,7 +2119,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControlSequential) {
std::move(shortHeader2), std::move(shortHeader2),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder2.encodePacketHeader().hasError()); ASSERT_FALSE(builder2.encodePacketHeader().hasError());
scheduler.writeStreams(builder2); ASSERT_FALSE(scheduler.writeStreams(builder2).hasError());
auto packet2 = std::move(builder2).buildPacket().packet; auto packet2 = std::move(builder2).buildPacket().packet;
ASSERT_FALSE( ASSERT_FALSE(
updateConnection( updateConnection(
@ -2156,7 +2170,7 @@ TEST_P(QuicPacketSchedulerTest, MultipleStreamsRunOutOfFlowControl) {
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_TRUE(builder1.encodePacketHeader()); ASSERT_TRUE(builder1.encodePacketHeader());
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
auto packet1 = std::move(builder1).buildPacket().packet; auto packet1 = std::move(builder1).buildPacket().packet;
ASSERT_TRUE(updateConnection( ASSERT_TRUE(updateConnection(
conn, std::nullopt, packet1, Clock::now(), 1200, 0, false /* isDSR */)); conn, std::nullopt, packet1, Clock::now(), 1200, 0, false /* isDSR */));
@ -2191,7 +2205,7 @@ TEST_P(QuicPacketSchedulerTest, MultipleStreamsRunOutOfFlowControl) {
std::move(shortHeader2), std::move(shortHeader2),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_TRUE(builder2.encodePacketHeader()); ASSERT_TRUE(builder2.encodePacketHeader());
scheduler.writeStreams(builder2); ASSERT_FALSE(scheduler.writeStreams(builder2).hasError());
auto packet2 = std::move(builder2).buildPacket().packet; auto packet2 = std::move(builder2).buildPacket().packet;
ASSERT_TRUE(updateConnection( ASSERT_TRUE(updateConnection(
conn, std::nullopt, packet2, Clock::now(), 1000, 0, false /* isDSR */)); conn, std::nullopt, packet2, Clock::now(), 1000, 0, false /* isDSR */));
@ -2238,7 +2252,7 @@ TEST_P(QuicPacketSchedulerTest, RunOutFlowControlDuringStreamWrite) {
std::move(shortHeader1), std::move(shortHeader1),
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder1.encodePacketHeader().hasError()); ASSERT_FALSE(builder1.encodePacketHeader().hasError());
scheduler.writeStreams(builder1); ASSERT_FALSE(scheduler.writeStreams(builder1).hasError());
auto packet1 = std::move(builder1).buildPacket().packet; auto packet1 = std::move(builder1).buildPacket().packet;
ASSERT_FALSE( ASSERT_FALSE(
updateConnection( updateConnection(
@ -2293,7 +2307,7 @@ TEST_P(QuicPacketSchedulerTest, WritingFINFromBufWithBufMetaFirst) {
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder.encodePacketHeader().hasError()); ASSERT_FALSE(builder.encodePacketHeader().hasError());
StreamFrameScheduler scheduler(conn); StreamFrameScheduler scheduler(conn);
scheduler.writeStreams(builder); ASSERT_FALSE(scheduler.writeStreams(builder).hasError());
auto packet = std::move(builder).buildPacket().packet; auto packet = std::move(builder).buildPacket().packet;
ASSERT_EQ(1, packet.frames.size()); ASSERT_EQ(1, packet.frames.size());
auto streamFrame = *packet.frames[0].asWriteStreamFrame(); auto streamFrame = *packet.frames[0].asWriteStreamFrame();
@ -2331,7 +2345,7 @@ TEST_P(QuicPacketSchedulerTest, NoFINWriteWhenBufMetaWrittenFIN) {
conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
ASSERT_FALSE(builder.encodePacketHeader().hasError()); ASSERT_FALSE(builder.encodePacketHeader().hasError());
StreamFrameScheduler scheduler(conn); StreamFrameScheduler scheduler(conn);
scheduler.writeStreams(builder); ASSERT_FALSE(scheduler.writeStreams(builder).hasError());
auto packet = std::move(builder).buildPacket().packet; auto packet = std::move(builder).buildPacket().packet;
EXPECT_EQ(1, packet.frames.size()); EXPECT_EQ(1, packet.frames.size());
auto streamFrame = *packet.frames[0].asWriteStreamFrame(); auto streamFrame = *packet.frames[0].asWriteStreamFrame();
@ -2850,7 +2864,8 @@ TEST_P(QuicPacketSchedulerTest, PausedPriorityEnabled) {
// Should write frames for only regular stream. // Should write frames for only regular stream.
auto builder = setupMockPacketBuilder(); auto builder = setupMockPacketBuilder();
scheduler.writeStreams(*builder); auto result4 = scheduler.writeStreams(*builder);
ASSERT_FALSE(result4.hasError());
verifyStreamFrames(*builder, {regularFrame}); verifyStreamFrames(*builder, {regularFrame});
@ -2859,7 +2874,8 @@ TEST_P(QuicPacketSchedulerTest, PausedPriorityEnabled) {
// Unpause the stream. Expect the scheduleor to write the data. // Unpause the stream. Expect the scheduleor to write the data.
conn.streamManager->setStreamPriority(pausedStreamId, kSequentialPriority); conn.streamManager->setStreamPriority(pausedStreamId, kSequentialPriority);
scheduler.writeStreams(*builder); auto result2 = scheduler.writeStreams(*builder);
ASSERT_FALSE(result2.hasError());
verifyStreamFrames(*builder, {pausedFrame}); verifyStreamFrames(*builder, {pausedFrame});
@ -2885,7 +2901,8 @@ TEST_P(QuicPacketSchedulerTest, PausedPriorityDisabled) {
auto regularFrame = writeDataToStream(conn, regularStreamId, "regular_data"); auto regularFrame = writeDataToStream(conn, regularStreamId, "regular_data");
auto builder = setupMockPacketBuilder(); auto builder = setupMockPacketBuilder();
scheduler.writeStreams(*builder); auto result = scheduler.writeStreams(*builder);
ASSERT_FALSE(result.hasError());
verifyStreamFrames(*builder, {regularFrame, pausedFrame}); verifyStreamFrames(*builder, {regularFrame, pausedFrame});
} }