Conversation
|
An automated preview of the documentation is available at https://119.beast2.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-01-05 16:58:49 UTC |
MungoG
force-pushed
the
issue-55-async-body-write-stream
branch
from
1823aa8 to
e14d19e
Compare
| private: | ||
| AsyncWriteStream& stream_; | ||
| http_proto::serializer& sr_; | ||
| http_proto::serializer::stream& srs_; |
There was a problem hiding this comment.
I don't think this is right, the user should not have to create serializer::stream and pass it in.
There was a problem hiding this comment.
This is left as is for the time being per the slack discussion.
There was a problem hiding this comment.
Correct, however, body_write_stream should take ownership of serializer::stream.
MungoG
force-pushed
the
issue-55-async-body-write-stream
branch
from
283da17 to
32e2162
Compare
MungoG
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #119 +/- ##
===========================================
+ Coverage 48.59% 51.11% +2.51%
===========================================
Files 36 37 +1
Lines 1457 1528 +71
===========================================
+ Hits 708 781 +73
+ Misses 749 747 -2
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| namespace detail { | ||
|
|
||
| template<class AsyncWriteStream> | ||
| class body_write_stream_close_op : public asio::coroutine |
There was a problem hiding this comment.
Couldn't composed operations simply point back to body_write_stream?
There was a problem hiding this comment.
done - they now hold a reference to body_write_stream
| AsyncWriteStream& stream_; | ||
| ConstBufferSequence cb_; | ||
| http_proto::serializer& sr_; | ||
| http_proto::serializer::stream& srs_; | ||
| system::error_code& saved_ec_; | ||
| std::size_t bytes_; |
There was a problem hiding this comment.
Couldn't composed operations simply point back to body_write_stream?
There was a problem hiding this comment.
done - they now hold a reference to body_write_stream
| if(sr_.is_done() || | ||
| !srs_.is_open()) | ||
| ec = asio::error::not_connected; |
There was a problem hiding this comment.
Checking for sr_.is_done() and !srs_.is_open() seems unnecessary. Could you please explain the logic behind it?
There was a problem hiding this comment.
These are preconditions, so changed to asserts.
| { | ||
| self.reset_cancellation_state(asio::enable_total_cancellation()); | ||
|
|
||
| bytes_ = 0; |
There was a problem hiding this comment.
Unnecessary (already initialized in the ctor).
| BOOST_ASIO_HANDLER_LOCATION( | ||
| (__FILE__, __LINE__, "async_write_some")); | ||
| beast2::async_write_some(stream_, sr_, std::move(self)); | ||
| } |
There was a problem hiding this comment.
I think the loop could be simplified in a way that the second call to async_write_some won't be needed. like:
for(;;)
{
bytes_ = asio::buffer_copy(srs_.prepare(), cb_);
srs_.commit(bytes_);
BOOST_ASIO_CORO_YIELD
{
BOOST_ASIO_HANDLER_LOCATION(
(__FILE__, __LINE__, "async_write_some"));
async_write_some(stream_, sr_, std::move(self));
}
if(ec.failed())
{
if(bytes_ != 0)
{
saved_ec_ = ec;
ec = {};
}
break;
}
if(bytes_ != 0)
break;
if(!!self.cancelled())
{
ec = asio::error::operation_aborted;
break;
}
}There was a problem hiding this comment.
updated per this suggestion.
| http::serializer::stream srs, | ||
| std::string const& body, | ||
| std::string const& expected_msg) | ||
| { |
There was a problem hiding this comment.
I'm not sure mixing coroutine tests here is very useful, because we code against the Asio interface and expect it to work with any kind of completion token. This feels more like a capy test.
| coro_close(body_write_stream<Stream>& bws) | ||
| { | ||
| return capy::make_async_op<system::error_code>( | ||
| bws.async_close(asio::deferred)); | ||
| } |
There was a problem hiding this comment.
I think we should eventually be able to write:
co_await bws.async_close(capy::op);without defining any any coroutine function.
MungoG
force-pushed
the
issue-55-async-body-write-stream
branch
from
f1a92b8 to
6d83768
Compare
4 participants
No description provided.