Fix tcp_server::bind() to not throw despite returning error_code#105
Fix tcp_server::bind() to not throw despite returning error_code#105MungoG merged 1 commit intocppalliance:developfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #105 +/- ##
===========================================
- Coverage 80.42% 80.42% -0.01%
===========================================
Files 64 64
Lines 5401 5406 +5
===========================================
+ Hits 4344 4348 +4
- Misses 1057 1058 +1
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
An automated preview of the documentation is available at https://105.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-04 19:20:37 UTC |
|
GCOVR code coverage report https://105.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-04 19:22:10 UTC |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/tcp_acceptor.hpp (1)
192-205:⚠️ Potential issue | 🟡 MinorDocument notable error conditions and no-throw behavior.
Now that
listen()returnsstd::error_code, the docs should list key error conditions (e.g.,operation_not_supportedwhen no acceptor_service is present) and explicitly state the exception behavior.As per coding guidelines, docstrings should include error conditions and `@throws` documentation.✍️ Suggested doc augmentation
@@ - `@return` An error code indicating success or the reason for failure. - A default-constructed error code indicates success. + `@return` An error code indicating success or the reason for failure. + A default-constructed error code indicates success. + + `@par` Error Conditions + - operation_not_supported: the acceptor service is unavailable in the context. + - errc::address_in_use, errc::permission_denied, etc., depending on bind/listen failure. + + `@throws` + No exceptions are thrown during normal operation.
bc2e92c to
3952d11
Compare
tcp_server::bind() could throw because tcp_acceptor::listen() had
throwing overloads. This changes listen() to return [[nodiscard]]
std::error_code, allowing bind() to properly propagate errors without
throwing.
API change for tcp_acceptor::listen():
Before: void listen(endpoint, int backlog = 128) // throws
void listen(endpoint, std::error_code&) // out-param
void listen(endpoint, int, std::error_code&) // out-param
After: [[nodiscard]] std::error_code listen(endpoint, int backlog = 128)
The [[nodiscard]] attribute ensures callers cannot accidentally ignore
errors. This aligns with signal_set and socket options which already
use non-throwing return values.
Changes:
- Fix tcp_server::bind() to capture and return listen() error code
- Consolidate tcp_acceptor::listen() to single non-throwing overload
- Document error conditions (address_in_use, address_not_available,
permission_denied, operation_not_supported) and @throws Nothing
- Update test utilities (socket_pair, mocket) with error checks
- Update all unit tests (~20 call sites) to check return values
- Update documentation (tcp_acceptor.adoc, tls.adoc, signals.adoc,
endpoints.adoc) with new error-handling pattern
3952d11 to
ba7f418
Compare
tcp_server::bind() was documented to return std::error_code but internally called the throwing tcp_acceptor::listen() overload, violating its API contract.
Add non-throwing listen() overloads to tcp_acceptor that take std::error_code& as the last parameter, following Boost.Asio conventions. The throwing overload now delegates to the non-throwing implementation.
Changes:
Summary by CodeRabbit