diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 297baeba8..3df19e991 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -319,7 +319,10 @@ FrameScheduler::scheduleFramesForPacket( pingFrameScheduler_->writePing(wrapper); } if (streamFrameScheduler_ && streamFrameScheduler_->hasPendingData()) { - streamFrameScheduler_->writeStreams(wrapper); + auto result = streamFrameScheduler_->writeStreams(wrapper); + if (result.hasError()) { + return folly::makeUnexpected(result.error()); + } } if (datagramFrameScheduler_ && datagramFrameScheduler_->hasPendingDatagramFrames()) { @@ -386,7 +389,7 @@ folly::StringPiece FrameScheduler::name() const { return name_; } -bool StreamFrameScheduler::writeStreamLossBuffers( +folly::Expected StreamFrameScheduler::writeStreamLossBuffers( PacketBuilderInterface& builder, QuicStreamState& stream) { bool wroteStreamFrame = false; @@ -404,8 +407,7 @@ bool StreamFrameScheduler::writeStreamLossBuffers( std::nullopt /* skipLenHint */, stream.groupId); if (res.hasError()) { - throw QuicInternalException( - res.error().message, *res.error().code.asLocalErrorCode()); + return folly::makeUnexpected(res.error()); } auto dataLen = *res; if (dataLen) { @@ -426,19 +428,28 @@ bool StreamFrameScheduler::writeStreamLossBuffers( StreamFrameScheduler::StreamFrameScheduler(QuicConnectionStateBase& conn) : conn_(conn) {} -StreamFrameScheduler::StreamWriteResult StreamFrameScheduler::writeSingleStream( +folly::Expected +StreamFrameScheduler::writeSingleStream( PacketBuilderInterface& builder, QuicStreamState& stream, uint64_t& connWritableBytes) { StreamWriteResult result = StreamWriteResult::NOT_LIMITED; 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; } } if (stream.hasWritableData(true)) { 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; } result = (connWritableBytes == 0) ? StreamWriteResult::CONN_FC_LIMITED @@ -450,7 +461,7 @@ StreamFrameScheduler::StreamWriteResult StreamFrameScheduler::writeSingleStream( return result; } -StreamId StreamFrameScheduler::writeStreamsHelper( +folly::Expected StreamFrameScheduler::writeStreamsHelper( PacketBuilderInterface& builder, const std::set& writableStreams, StreamId nextScheduledStream, @@ -466,7 +477,10 @@ StreamId StreamFrameScheduler::writeStreamsHelper( auto stream = conn_.streamManager->findStream(*writableStreamItr); CHECK(stream); 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; } writableStreamItr++; @@ -477,7 +491,8 @@ StreamId StreamFrameScheduler::writeStreamsHelper( return *writableStreamItr; } -void StreamFrameScheduler::writeStreamsHelper( +folly::Expected +StreamFrameScheduler::writeStreamsHelper( PacketBuilderInterface& builder, deprecated::PriorityQueue& writableStreams, uint64_t& connWritableBytes, @@ -498,12 +513,15 @@ void StreamFrameScheduler::writeStreamsHelper( auto stream = CHECK_NOTNULL(conn_.streamManager->findStream(streamId)); if (!stream->hasSchedulableData() && stream->hasSchedulableDsr()) { // We hit a DSR stream - return; + return folly::unit; } CHECK(stream) << "streamId=" << streamId << "inc=" << uint64_t(level.incremental); - if (writeSingleStream(builder, *stream, connWritableBytes) == - StreamWriteResult::PACKET_FULL) { + auto writeResult = writeSingleStream(builder, *stream, connWritableBytes); + if (writeResult.hasError()) { + return folly::makeUnexpected(writeResult.error()); + } + if (writeResult.value() == StreamWriteResult::PACKET_FULL) { break; } auto remainingSpaceAfter = builder.remainingSpaceInPkt(); @@ -513,13 +531,15 @@ void StreamFrameScheduler::writeStreamsHelper( bool forceNext = remainingSpaceAfter > 0; level.iterator->next(forceNext); if (streamPerPacket) { - return; + return folly::unit; } } while (!level.iterator->end()); } + return folly::unit; } -void StreamFrameScheduler::writeStreamsHelper( +folly::Expected +StreamFrameScheduler::writeStreamsHelper( PacketBuilderInterface& builder, PriorityQueue& writableStreams, uint64_t& connWritableBytes, @@ -538,13 +558,16 @@ void StreamFrameScheduler::writeStreamsHelper( auto stream = CHECK_NOTNULL(conn_.streamManager->findStream(streamId)); if (!stream->hasSchedulableData() && stream->hasSchedulableDsr()) { // We hit a DSR stream - return; + return folly::unit; } CHECK(stream) << "streamId=" << streamId; // TODO: this is counting STREAM frame overhead against the stream itself auto lastWriteBytes = builder.remainingSpaceInPkt(); 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; } auto remainingSpaceAfter = builder.remainingSpaceInPkt(); @@ -554,7 +577,7 @@ void StreamFrameScheduler::writeStreamsHelper( // we should erase the stream from writableStreams, the caller can rollback // the transaction if the packet write fails if (remainingSpaceAfter > 0) { - if (writeResult == StreamWriteResult::CONN_FC_LIMITED) { + if (writeResult.value() == StreamWriteResult::CONN_FC_LIMITED) { conn_.streamManager->addConnFCBlockedStream(streamId); } writableStreams.erase(id); @@ -562,44 +585,56 @@ void StreamFrameScheduler::writeStreamsHelper( writableStreams.consume(lastWriteBytes); } if (streamPerPacket) { - return; + return folly::unit; } } + return folly::unit; } -void StreamFrameScheduler::writeStreams(PacketBuilderInterface& builder) { +folly::Expected StreamFrameScheduler::writeStreams( + PacketBuilderInterface& builder) { DCHECK(conn_.streamManager->hasWritable()); uint64_t connWritableBytes = getSendConnFlowControlBytesWire(conn_); // Write the control streams first as a naive binary priority mechanism. const auto& controlWriteQueue = conn_.streamManager->controlWriteQueue(); if (!controlWriteQueue.empty()) { - conn_.schedulingState.nextScheduledControlStream = writeStreamsHelper( + auto result = writeStreamsHelper( builder, controlWriteQueue, conn_.schedulingState.nextScheduledControlStream, connWritableBytes, conn_.transportSettings.streamFramePerPacket); + if (result.hasError()) { + return folly::makeUnexpected(result.error()); + } + conn_.schedulingState.nextScheduledControlStream = result.value(); } auto* oldWriteQueue = conn_.streamManager->oldWriteQueue(); QuicStreamState* nextStream{nullptr}; if (oldWriteQueue) { if (!oldWriteQueue->empty()) { - writeStreamsHelper( + auto result = writeStreamsHelper( builder, *oldWriteQueue, connWritableBytes, conn_.transportSettings.streamFramePerPacket); + if (result.hasError()) { + return folly::makeUnexpected(result.error()); + } auto streamId = oldWriteQueue->getNextScheduledStream(); nextStream = conn_.streamManager->findStream(streamId); } } else { auto& writeQueue = conn_.streamManager->writeQueue(); if (!writeQueue.empty()) { - writeStreamsHelper( + auto result = writeStreamsHelper( builder, writeQueue, connWritableBytes, conn_.transportSettings.streamFramePerPacket); + if (result.hasError()) { + return folly::makeUnexpected(result.error()); + } if (!writeQueue.empty()) { auto id = writeQueue.peekNextScheduledID(); CHECK(id.isStreamID()); @@ -615,6 +650,7 @@ void StreamFrameScheduler::writeStreams(PacketBuilderInterface& builder) { if (nextStream && !nextStream->hasSchedulableData()) { nextStreamDsr_ = true; } + return folly::unit; } bool StreamFrameScheduler::hasPendingData() const { @@ -624,7 +660,7 @@ bool StreamFrameScheduler::hasPendingData() const { getSendConnFlowControlBytesWire(conn_) > 0)); } -bool StreamFrameScheduler::writeStreamFrame( +folly::Expected StreamFrameScheduler::writeStreamFrame( PacketBuilderInterface& builder, QuicStreamState& stream, uint64_t& connWritableBytes) { @@ -653,8 +689,7 @@ bool StreamFrameScheduler::writeStreamFrame( std::nullopt /* skipLenHint */, stream.groupId); if (res.hasError()) { - throw QuicInternalException( - res.error().message, *res.error().code.asLocalErrorCode()); + return folly::makeUnexpected(res.error()); } auto dataLen = *res; if (!dataLen) { diff --git a/quic/api/QuicPacketScheduler.h b/quic/api/QuicPacketScheduler.h index ceace1a27..6907e95d2 100644 --- a/quic/api/QuicPacketScheduler.h +++ b/quic/api/QuicPacketScheduler.h @@ -77,13 +77,14 @@ class StreamFrameScheduler { * Return: the first boolean indicates if at least one Blocked frame * is written into the packet by writeStreams function. */ - void writeStreams(PacketBuilderInterface& builder); + [[nodiscard]] folly::Expected writeStreams( + PacketBuilderInterface& builder); bool hasPendingData() const; private: // Return true if this stream wrote some data - bool writeStreamLossBuffers( + [[nodiscard]] folly::Expected writeStreamLossBuffers( PacketBuilderInterface& builder, QuicStreamState& stream); @@ -100,25 +101,25 @@ class StreamFrameScheduler { * flow control limited, or not limited by connection flow control. */ enum class StreamWriteResult { PACKET_FULL, NOT_LIMITED, CONN_FC_LIMITED }; - StreamWriteResult writeSingleStream( + [[nodiscard]] folly::Expected writeSingleStream( PacketBuilderInterface& builder, QuicStreamState& stream, uint64_t& connWritableBytes); - StreamId writeStreamsHelper( + [[nodiscard]] folly::Expected writeStreamsHelper( PacketBuilderInterface& builder, const std::set& writableStreams, StreamId nextScheduledStream, uint64_t& connWritableBytes, bool streamPerPacket); - void writeStreamsHelper( + [[nodiscard]] folly::Expected writeStreamsHelper( PacketBuilderInterface& builder, deprecated::PriorityQueue& writableStreams, uint64_t& connWritableBytes, bool streamPerPacket); - void writeStreamsHelper( + [[nodiscard]] folly::Expected writeStreamsHelper( PacketBuilderInterface& builder, PriorityQueue& writableStreams, uint64_t& connWritableBytes, @@ -130,7 +131,7 @@ class StreamFrameScheduler { * * Return: A boolean indicates if write is successful. */ - bool writeStreamFrame( + [[nodiscard]] folly::Expected writeStreamFrame( PacketBuilderInterface& builder, QuicStreamState& stream, uint64_t& connWritableBytes); diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index cf4c43a6a..336f90d15 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -512,7 +512,9 @@ TEST_P(QuicPacketSchedulerTest, CryptoSchedulerOnlySingleLossFits) { ChainedByteRangeHead(helloBuf), 0, false); conn.cryptoState->handshakeStream.lossBuffer.emplace_back( 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) { @@ -1519,7 +1521,9 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) { auto f3 = writeDataToStream(conn, stream3, "some data"); auto builder = setupMockPacketBuilder(); - scheduler.writeStreams(*builder); + auto result4 = scheduler.writeStreams(*builder); + ASSERT_FALSE(result4.hasError()); + verifyStreamFrames(*builder, {f1, f2, f3}); if (GetParam()) { EXPECT_TRUE(conn.streamManager->writeQueue().empty()); @@ -1544,11 +1548,13 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) { // write a normal size packet from stream1 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. auto builder2 = setupMockPacketBuilder(); - scheduler.writeStreams(*builder2); + auto result2 = scheduler.writeStreams(*builder2); + ASSERT_FALSE(result2.hasError()); verifyStreamFrames(*builder2, {f2, f3, f1}); } @@ -1571,16 +1577,17 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinNextsPer) { // stream1 again. auto builder2 = setupMockPacketBuilder({1500, 0, 1400, 0, 1300, 1100, 1000, 0}); - scheduler.writeStreams(*builder2); + auto result2 = scheduler.writeStreams(*builder2); + ASSERT_FALSE(result2.hasError()); builder2->advanceRemaining(); ASSERT_EQ(nextScheduledStreamID(conn), stream1); ASSERT_EQ(builder2->frames_.size(), 1); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); ASSERT_EQ(builder2->frames_.size(), 2); ASSERT_EQ(nextScheduledStreamID(conn), stream2); builder2->advanceRemaining(); - scheduler.writeStreams(*builder2); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {stream1, stream1, stream2, stream3, stream1}); } @@ -1601,16 +1608,16 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinStreamPerPacket) { // Write a normal size packet from stream1 auto builder = createPacketBuilder(conn); - scheduler.writeStreams(builder); + ASSERT_FALSE(scheduler.writeStreams(builder).hasError()); EXPECT_EQ(nextScheduledStreamID(conn), stream2); // Should write frames for stream2, stream3, followed by stream1 again. auto builder2 = setupMockPacketBuilder(); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f2}); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f3}); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f1}); } @@ -1656,20 +1663,21 @@ TEST_P( // Write a normal size packet from stream1 auto builder1 = createPacketBuilder(conn); - scheduler.writeStreams(builder1); + auto result = scheduler.writeStreams(builder1); + ASSERT_FALSE(result.hasError()); EXPECT_EQ(nextScheduledStreamID(conn), stream2); // Should write frames for stream2, stream3, followed by an empty write. auto builder2 = setupMockPacketBuilder(); ASSERT_TRUE(scheduler.hasPendingData()); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); ASSERT_EQ(builder2->frames_.size(), 1); ASSERT_TRUE(scheduler.hasPendingData()); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); ASSERT_EQ(builder2->frames_.size(), 2); EXPECT_FALSE(scheduler.hasPendingData()); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f2, f3}); } @@ -1690,13 +1698,14 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerSequential) { // Write a normal size packet from stream1 auto builder1 = createPacketBuilder(conn); - scheduler.writeStreams(builder1); + auto result = scheduler.writeStreams(builder1); + ASSERT_FALSE(result.hasError()); EXPECT_EQ(nextScheduledStreamID(conn), stream1); // Should write frames for stream1, stream2, stream3, in that order. auto builder2 = setupMockPacketBuilder(); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f1, f2, f3}); } @@ -1719,13 +1728,14 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerSequentialDefault) { // Write a normal size packet from stream1 auto builder1 = createPacketBuilder(conn); - scheduler.writeStreams(builder1); + auto result = scheduler.writeStreams(builder1); + ASSERT_FALSE(result.hasError()); EXPECT_EQ(nextScheduledStreamID(conn), stream1); // Should write frames for stream1, stream2, stream3, in that order. auto builder2 = setupMockPacketBuilder(); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f1, f2, f3}); } @@ -1753,7 +1763,8 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) { // This writes a normal size packet with 2, 4, 1 auto builder1 = createPacketBuilder(conn); - scheduler.writeStreams(builder1); + auto result = scheduler.writeStreams(builder1); + ASSERT_FALSE(result.hasError()); EXPECT_EQ(nextScheduledStreamID(conn), stream3); 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 // Should write frames for stream2, stream4, followed by stream 3 then 1. auto builder2 = setupMockPacketBuilder(); - scheduler.writeStreams(*builder2); + ASSERT_FALSE(scheduler.writeStreams(*builder2).hasError()); verifyStreamFrames(*builder2, {f2, f4, f3, f1}); @@ -1782,7 +1793,7 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) { writeDataToStream(conn, stream1, "some data"); auto builder1 = createPacketBuilder(conn); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); if (GetParam()) { EXPECT_TRUE(conn.streamManager->writeQueue().empty()); @@ -1803,7 +1814,9 @@ TEST_P(QuicPacketSchedulerTest, StreamFrameSchedulerRemoveOne) { auto f2 = writeDataToStream(conn, stream2, "some data"); auto builder = setupMockPacketBuilder(); - scheduler.writeStreams(*builder); + auto result4 = scheduler.writeStreams(*builder); + ASSERT_FALSE(result4.hasError()); + verifyStreamFrames(*builder, {f1, f2}); // 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->findStream(stream2)); - scheduler.writeStreams(*builder); + auto result = scheduler.writeStreams(*builder); + ASSERT_FALSE(result.hasError()); ASSERT_EQ(builder->frames_.size(), 1); verifyStreamFrames(*builder, {f2}); } @@ -1918,7 +1932,7 @@ TEST_P(QuicPacketSchedulerTest, HighPriNewDataBeforeLowPriLossData) { conn, highPriStreamId, buildRandomInputData(conn.udpSendPacketLen * 10)); auto builder1 = createPacketBuilder(conn); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); auto packet = std::move(builder1).buildPacket().packet; EXPECT_EQ(1, packet.frames.size()); @@ -1952,7 +1966,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) { std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder1.encodePacketHeader().hasError()); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); auto packet1 = std::move(builder1).buildPacket().packet; ASSERT_FALSE( updateConnection( @@ -1982,7 +1996,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControl) { std::move(shortHeader2), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder2.encodePacketHeader().hasError()); - scheduler.writeStreams(builder2); + ASSERT_FALSE(scheduler.writeStreams(builder2).hasError()); auto packet2 = std::move(builder2).buildPacket().packet; ASSERT_FALSE( updateConnection( @@ -2030,7 +2044,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControlIgnoreDSR) { std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder1.encodePacketHeader().hasError()); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); auto packet1 = std::move(builder1).buildPacket().packet; ASSERT_FALSE( updateConnection( @@ -2075,7 +2089,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControlSequential) { std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder1.encodePacketHeader().hasError()); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); auto packet1 = std::move(builder1).buildPacket().packet; ASSERT_FALSE( updateConnection( @@ -2105,7 +2119,7 @@ TEST_P(QuicPacketSchedulerTest, WriteLossWithoutFlowControlSequential) { std::move(shortHeader2), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder2.encodePacketHeader().hasError()); - scheduler.writeStreams(builder2); + ASSERT_FALSE(scheduler.writeStreams(builder2).hasError()); auto packet2 = std::move(builder2).buildPacket().packet; ASSERT_FALSE( updateConnection( @@ -2156,7 +2170,7 @@ TEST_P(QuicPacketSchedulerTest, MultipleStreamsRunOutOfFlowControl) { std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_TRUE(builder1.encodePacketHeader()); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); auto packet1 = std::move(builder1).buildPacket().packet; ASSERT_TRUE(updateConnection( conn, std::nullopt, packet1, Clock::now(), 1200, 0, false /* isDSR */)); @@ -2191,7 +2205,7 @@ TEST_P(QuicPacketSchedulerTest, MultipleStreamsRunOutOfFlowControl) { std::move(shortHeader2), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_TRUE(builder2.encodePacketHeader()); - scheduler.writeStreams(builder2); + ASSERT_FALSE(scheduler.writeStreams(builder2).hasError()); auto packet2 = std::move(builder2).buildPacket().packet; ASSERT_TRUE(updateConnection( conn, std::nullopt, packet2, Clock::now(), 1000, 0, false /* isDSR */)); @@ -2238,7 +2252,7 @@ TEST_P(QuicPacketSchedulerTest, RunOutFlowControlDuringStreamWrite) { std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder1.encodePacketHeader().hasError()); - scheduler.writeStreams(builder1); + ASSERT_FALSE(scheduler.writeStreams(builder1).hasError()); auto packet1 = std::move(builder1).buildPacket().packet; ASSERT_FALSE( updateConnection( @@ -2293,7 +2307,7 @@ TEST_P(QuicPacketSchedulerTest, WritingFINFromBufWithBufMetaFirst) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder.encodePacketHeader().hasError()); StreamFrameScheduler scheduler(conn); - scheduler.writeStreams(builder); + ASSERT_FALSE(scheduler.writeStreams(builder).hasError()); auto packet = std::move(builder).buildPacket().packet; ASSERT_EQ(1, packet.frames.size()); auto streamFrame = *packet.frames[0].asWriteStreamFrame(); @@ -2331,7 +2345,7 @@ TEST_P(QuicPacketSchedulerTest, NoFINWriteWhenBufMetaWrittenFIN) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); ASSERT_FALSE(builder.encodePacketHeader().hasError()); StreamFrameScheduler scheduler(conn); - scheduler.writeStreams(builder); + ASSERT_FALSE(scheduler.writeStreams(builder).hasError()); auto packet = std::move(builder).buildPacket().packet; EXPECT_EQ(1, packet.frames.size()); auto streamFrame = *packet.frames[0].asWriteStreamFrame(); @@ -2850,7 +2864,8 @@ TEST_P(QuicPacketSchedulerTest, PausedPriorityEnabled) { // Should write frames for only regular stream. auto builder = setupMockPacketBuilder(); - scheduler.writeStreams(*builder); + auto result4 = scheduler.writeStreams(*builder); + ASSERT_FALSE(result4.hasError()); verifyStreamFrames(*builder, {regularFrame}); @@ -2859,7 +2874,8 @@ TEST_P(QuicPacketSchedulerTest, PausedPriorityEnabled) { // Unpause the stream. Expect the scheduleor to write the data. conn.streamManager->setStreamPriority(pausedStreamId, kSequentialPriority); - scheduler.writeStreams(*builder); + auto result2 = scheduler.writeStreams(*builder); + ASSERT_FALSE(result2.hasError()); verifyStreamFrames(*builder, {pausedFrame}); @@ -2885,7 +2901,8 @@ TEST_P(QuicPacketSchedulerTest, PausedPriorityDisabled) { auto regularFrame = writeDataToStream(conn, regularStreamId, "regular_data"); auto builder = setupMockPacketBuilder(); - scheduler.writeStreams(*builder); + auto result = scheduler.writeStreams(*builder); + ASSERT_FALSE(result.hasError()); verifyStreamFrames(*builder, {regularFrame, pausedFrame}); }