Error normalization - #341
Merged
Merged
Conversation
…nc surface Applies the library-wide error model to every synchronous operation: expected runtime conditions travel through [[nodiscard]] returns — std::error_code, or io_result<T> when a payload rides along — while exceptions are reserved for precondition violations. One channel per operation. - File ops: open, resize, sync_data, sync_all, and assign return std::error_code; seek returns io_result<std::uint64_t> carrying the new position. Missing files, disk full, device I/O errors, data-driven seek offsets, and foreign handles are runtime conditions; size() and release(), whose only failure is misuse of a closed file, keep throwing. The implementation virtuals change shape and become noexcept across posix, io_uring, and IOCP. - open() on the four socket classes and both acceptors returns the code: descriptor exhaustion and address-family rejection are on par with bind's EADDRINUSE, so the whole setup sequence speaks one channel. Opening an already-open object stays a no-op reporting success; connect()'s auto-open forwards an open failure into the returned awaitable, which completes immediately instead of throwing from the initiator. - assign() returns the code: adopting a foreign descriptor is untrusted input, and the service layer already produced the code the wrapper was converting into an exception. On failure the caller retains the descriptor. - shutdown() returns the code: tcp and udp were silently discarding the underlying error, and the local sockets carried the asio-style dual overloads — throwing plus an error_code& out-param — which are removed. A peer that already disconnected is a normal runtime condition the caller may want to observe. udp_socket gains shutdown() across all backends — connected datagram sockets support it everywhere. - Every remaining sync error-code return is [[nodiscard]]: the tls_context setters, signal_set add/remove/clear, tcp_server and local_datagram_socket bind, and both connect_pair overloads. A silently dropped certificate-load failure is a latent bug; the attribute makes it a warning. Tests and doc snippets consume the returns they previously ignored.
A closed object reports errc::bad_file_descriptor everywhere, delivered through whichever channel the operation already uses: - bind() and listen() return the code instead of throwing logic_error, and become noexcept. - release(), available(), set_option(), and get_option() throw std::system_error carrying the code instead of logic_error — misuse and runtime failure share one exception type. - Async initiators (datagram send/recv, acceptor accept/wait, file read_some_at/write_some_at, and the native wrappers) no longer throw: the returned awaitable completes immediately with the code, matching how a racing close() already surfaces to in-flight operations. The op_base and native awaitables treat a pre-set error as ready-with-result. - close() is noexcept on every io object: idempotent, callable from destructors, and the descriptor is released even when the underlying close reports an error. Durability belongs to sync_*. - host_name() throws std::system_error rather than runtime_error. Genuine logic errors unrelated to object state — service not installed, launcher invoked twice, zero-sized thread pool — keep throwing logic_error. local_endpoint's out-param constructor — the library's last error_code& signature — is removed: an over-long path is a precondition violation with a public pre-check (max_path_length), so the throwing constructor reporting errc::filename_too_long is the whole API. The sockaddr conversion path applies the documented pre-check, since a foreign sun_path may legitimately exceed the cap. The lockdown suite pins the portable codes the normalized API promises across all reactor backends: closed-object completions and returns assert bad_file_descriptor by equality, genuine runtime failures are exercised (wrong-direction I/O, huge offsets, bind conflicts, pipe sync), deterministic corosio-generated codes assert equality while platform-varying codes assert only that a genuine error arrived, and deliberate discards use std::ignore. Lockdowns added from the coverage pass: io_context's documented invalid_argument on thread_pool_size < 1, and ipv4_address::to_buffer's length_error (matching the existing ipv6 test).
…ry backend Every backend completes read/write/wait on a closed object with bad_file_descriptor at the initiation entry, contracted codes compare against std::errc on every toolchain, and the openssl engine can no longer lose an error entirely. - reactor (epoll/select/kqueue): read/write on a never-registered socket completed through complete_io_op, which dereferenced the null descriptor scheduler — a segfault. The entry check completes before touching the kernel, and complete_io_op gets the same null-scheduler guard complete_wait_op already had. - io_uring: a closed acceptor's wait parked a waiter node the multishot accept machinery would never signal — a hang. The entry check posts an EBADF completion instead. - iocp: closed sockets reached WSARecv/WSASend/the wait reactor and reported WSAENOTSOCK or toolchain-dependent mappings. Entry checks complete with WSAEBADF (ERROR_INVALID_HANDLE for files), and iocp_make_err normalizes both spellings to bad_file_descriptor — MSVC maps ERROR_INVALID_HANDLE to invalid_argument on its own. - native_tcp_socket::connect pre-failed a closed socket instead of auto-opening like the base class and every sibling facade; it now opens with the family-matched protocol. - WSAEADDRINUSE, WSAEADDRNOTAVAIL, and ERROR_NEGATIVE_SEEK join the make_err contract list so bind and seek failures compare portably; the raw-pin test gates collapse to unconditional equality. - openssl: make_implementation swallowed an engine init failure by returning nullptr, which the constructor stored — every later operation dereferenced a null impl_, and the failed SSL_CTX cache made the crash permanent for the whole tls_context. Construction now only allocates; prepare() runs the deferred init on the first handshake (the wolfssl shape) so setup failures surface through the handshake completion, re-running check_context after the lazy init because the driver's fail-closed gate runs before the native context exists. perform() reports pre-handshake I/O instead of crashing; the OOM trigger itself remains fault-injection-only. The closed-wait suite is promoted from reactor-only to every backend and extended to read/write and all four socket classes; closed-object completions are locked by equality on files, native at-ops, and native acceptor waits. The random-access completion decode joins iocp_make_err — it was the one overlapped op decoding through plain make_err, which left ERROR_INVALID_HANDLE to MSVC's invalid_argument mapping instead of the contracted bad_file_descriptor. Coverage: acceptor wait(wait_type::error) parked and cancelled on both acceptor types — the io_uring error-poll submit path had no test.
…] initiators Closes the remaining channel inconsistencies the error-handling audit found and completes the discard guard on the async surface: - parse_endpoint, parse_ipv4_address, and parse_ipv6_address were the library's last payload out-params — and with the payload in the return value they are factories, so they take asio's name for the operation: make_endpoint, make_ipv4_address, make_ipv6_address, returning io_result<T> with a documented, equality-tested default-constructed payload on failure. The throwing constructors stay as the convenience over the factories; the parser bodies survive byte-identical as file-local helpers. - ipv4_address and ipv6_address string constructors threw plain std::invalid_argument while endpoint(string_view) threw std::system_error for the same condition; all three now throw system_error carrying the deterministic invalid_argument contract. - connect_pair called closed-ness a Precondition while its noexcept body returned raw EISCONN/WSAEISCONN; the rejection is now a documented expected condition generating errc::already_connected. - mocket::close() was the one close() returning an error code; verification moves to a [[nodiscard]] verify() and close() keeps the library-wide idempotent void noexcept contract. - set_verify_callback joins its three record-and-defer siblings on the void shape. - local_stream_acceptor gains the (ctx, ep, backlog) convenience constructor tcp_acceptor already had — open + bind + listen, no reuse_address (AF_UNIX conflicts are path collisions), no implicit unlink (deleting a file is not a safe constructor default). - Every awaitable-returning initiator is [[nodiscard]] — a discarded awaitable is a silent no-op — and cancel() is noexcept across the public surface, matching the not-actionable classification. - temp_socket_dir includes the collected filesystem error in its thrown message. - Deliberate discards are std::ignore = expr repository-wide (~195 sites: awaited io_results in tests, best-effort setsockopt in the reactor traits, the TLS engine driver) and unused names carry [[maybe_unused]] on the declaration (~225 sites: partially consumed structured bindings, signature-preserving parameters, #if-gated uses) — no void cast remains, so the two intents are never spelled alike. Coverage: make_endpoint's invalid-v4-with-port and unbracketed-v6 arms, detect_endpoint_format on empty input, two embedded-IPv4 octet validator arms, and local_stream_acceptor::open's already-open no-op. - host_name() returns io_result<std::string> — it was the one remaining throwing-only surface whose failure the caller cannot pre-check (gethostname is a runtime condition), which made it the last obstacle to fully exception-free use of the library. Errors now ride the return; only allocation can throw.
The code was more correct than its documentation; this closes the gap the error-handling audit measured: - tls_context @return clauses promised immediate validation the implementation defers to the first handshake's native context build; every setter now states the deferred model, and the file-loading setters distinguish file-read errors (returned now) from decode errors (surfacing at handshake). - release() documents its throwing channel on tcp_acceptor, local_stream_acceptor, stream_file, and random_access_file; wait() docs replace the "must be open" precondition with the deterministic closed-object completion the backends now guarantee; the type-erased read_some/write_some and native send_to/recv_from docs state it too. - reuse_port names the right channel, the io_context option constructors document their invalid_argument, the tcp_acceptor convenience constructor enumerates all throwing steps, native accept() documents its moved-from logic_error, and the TLS streams stop calling a moved-from object "valid but unspecified". - Every @code example models checked usage: no discarded [[nodiscard]] results, no comment-only if bodies, no open() before an auto-opening connect(). The rulebook states the doctrine in the present tense, and every former special case is now derived: rule 1's classification gains its third outcome — not-actionable reports nothing (void noexcept), with everything actionable available through an earlier return-channel operation (sync_* for durability, shutdown for orderly teardown) and a reported close() error rejected as an attractive nuisance whose invited retry is a double-close hazard under EINTR; rule 2 gains the constructor derivation (no return channel exists, so only misuse behind a public pre-check, wrapped codes of an abbreviated piecewise path, or root setup); and Composite Operations replaces Special Cases — a multi-step operation reports its first failure through its own single channel, deriving connect()'s auto-open reporting, the convenience constructors, the free corosio::connect, and the TLS deferred-configuration model. Rule 7 fixes std::errc as the only user vocabulary via the make_err normalization boundary. Rule 6 legislates the discard split: std::ignore for deliberate discards, [[maybe_unused]] for unused names, never a void cast. The guide pages join the contract: the error-handling page opens with the one-channel model and the deterministic corosio code table, and its io_result prose matches the type (a std::tuple alias — the members it described never existed); the http-client tutorial and UDP guide drop must-open-before-connect claims; the mocket pages teach the verify()-then-close() idiom; the acceptor page names the portable canceled condition. The error-handling guide gains an Avoiding Exceptions section mapping every throwing convenience to its exception-free spelling (or public pre-check), the throwing constructors cross-reference their alternatives, and the acceptor guide's throw enumeration matches the implementation.
|
An automated preview of the documentation is available at https://341.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-08-21 20:39:38 UTC |
|
GCOVR code coverage report https://341.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-08-21 20:54:58 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #341 +/- ##
===========================================
- Coverage 79.81% 79.67% -0.14%
===========================================
Files 96 96
Lines 5924 5815 -109
Branches 1209 1191 -18
===========================================
- Hits 4728 4633 -95
+ Misses 849 835 -14
Partials 347 347
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #261.
This PR completely normalizes all error handling in Corosio and introduced a error handling rulebook at
doc/error-handling-rulebook.md.The overview is: