Support glz::registry and http_router under -fno-exceptions, without hiding errors (#2265) - #2588
Open
stephenberry wants to merge 20 commits into
Open
Support glz::registry and http_router under -fno-exceptions, without hiding errors (#2265)#2588stephenberry wants to merge 20 commits into
stephenberry wants to merge 20 commits into
Conversation
…ns (#2265) The RPC registry (REPE, JSON-RPC, REST) and http_router could not compile under -fno-exceptions, and route-registration conflicts were swallowed to stderr. This makes both usable without exceptions while surfacing errors through return values rather than hiding them. http_router: - add_route/add return glz::expected<void, std::string> instead of throwing on a structural conflict (mismatched :param / *wildcard names, wildcard-not-last). - route()/stream()/websocket() throw on conflict when exceptions are enabled (restoring the documented route() contract) and record a queryable error under -fno-exceptions. Nothing is swallowed to stderr or aborted. - New non-throwing try_route() plus has_route_error()/route_error()/clear_route_error(). registry: - The three user-handler try/catch sites are guarded by __cpp_exceptions. Glaze's own (de)serialization reports via error codes, so the catch only guarded user throws; under -fno-exceptions there is nothing to catch. - New non-throwing try_on() (object and merge overloads) returning glz::expected<void, std::string>. Handlers can fail a request without throwing by returning glz::expected<T, E>; write_response translates the error into the protocol's representation: - JSON-RPC: glz::rpc::error (full fidelity), rpc::error_e, error_ctx, or a string (mapped to internal error -32603). - REPE: glz::error_code, error_ctx, or a string (mapped to error_code::parse_error, matching the code used when a thrown handler is caught). Tests build the registry in both exceptions and -fno-exceptions configurations (glaze_BUILD_REGISTRY_NOEXCEPT_TESTS) and verify identical behavior. Docs updated. Closes #2265.
…TTP responses (#2265) Reflected REST endpoints serialized a handler's return value directly via response::body<Opts>, so a handler returning glz::expected<T, E> emitted the error as a 200 body ({"unexpected": ...}) instead of failing the request. Under -fno-exceptions such a handler had no failure channel at all, while the REPE and JSON-RPC registries already translate glz::expected errors into protocol errors. Add the REST peer of repe::/jsonrpc::set_handler_error: on a glz::expected the success value is serialized (204 for expected<void, E>) and the error is mapped to an HTTP status plus a glz::http_error body. error_code maps to a sensible status, string/error_ctx map to 500, and the new glz::http_error type gives handlers explicit status control. rpc::error is intentionally not accepted, since glaze/net must not depend on the glaze/ext JSON-RPC types. Response schemas now unwrap expected<T, E> to T so OpenAPI describes the value, not the wrapper.
…nAPI (#2265) A reflected REST handler that returns glz::expected now fails with a glz::http_error body, but the generated OpenAPI spec still advertised only a 200 success response, under-describing the contract on the error paths. Record the error response in route_spec when a handler returns glz::expected: the body is the glz::http_error schema, keyed "500" when the status is fixed (string/error_ctx errors) and "default" when it depends on the error value (error_code maps to several statuses, glz::http_error carries any). The OpenAPI generator emits this response and registers the http_error schema in components. The success schema already unwraps expected<T, E> to T, so 200 describes the value rather than the wrapper. Not addressed here (pre-existing, orthogonal): void/expected<void, E> endpoints return 204 at runtime but are still documented as 200, since route_spec does not yet record the success status code.
…#2265) void / expected<void, E> handlers and update (PUT) endpoints reply 204 at runtime but the generated OpenAPI spec hardcoded a 200 success response for every route. Record the success status on route_spec (200 with a body, 204 for No Content) at the single point where the spec is built: every void-success endpoint already passes ResponseType = void there, so spec.success_status = is_void ? 204 : 200 covers update endpoints, void functions, and expected<void, E> uniformly. The OpenAPI generator keys the success response on that status and describes 204 as "No Content".
…o-exceptions (#2265) The try_on() comment claimed it 'behaves identically with and without exceptions'. That holds for the returned error (always the first conflict) but not for the resulting route table: under exceptions the first conflict aborts the remaining registration, whereas under -fno-exceptions the conflict is recorded and later routes still register. Document the distinction and note it is unreachable through reflected registration (which emits only literal paths that never structurally conflict).
No test returned glz::expected<T, glz::error_ctx>, so the error_ctx arm of set_handler_error was discarded by 'if constexpr' and never instantiated in any protocol (REPE, JSON-RPC, REST) - a misnamed field would have compiled clean. Add one error_ctx-returning handler and one assertion-bearing test per protocol so each arm is instantiated and its field access exercised.
…orting (#2265) try_route() let normal routes report a structural conflict by return value under -fno-exceptions, but stream()/websocket() conflicts were observable only via the route_error() side-channel. Add try_stream()/try_websocket() as exact mirrors of try_route() (route_table::add() already returns the result), so all three route kinds share one return-value channel. Document the throw/try contract on stream()/websocket() and cover both new methods with tests.
A reflected handler returning glz::expected<T, E> used to have its result
serialized directly, so an error was delivered as a successful response with a
{"unexpected": <E>} body. The registry now intercepts glz::expected and maps an
error to a protocol error (REPE/JSON-RPC error or a non-2xx HTTP status). The
success path is unchanged. Document this in the no-exceptions guide.
…#2265) - REPE/JSON-RPC: apply the glz::expected error translation only to handler results, not to registered data members. The is_expected branch lived in the shared write_response, so reading a data member of type glz::expected was silently reinterpreted as a protocol error. Extract write_handler_result for the four function/member-function call sites and revert write_response to plain serialization, matching how REST already separates the two; a stored glz::expected member again serializes its {"unexpected": ...} shape. - http_router: try_route/try_stream/try_websocket now report conflicts purely by return value and no longer write the route_error() side-channel, which a return-value caller never reads and which (first-wins) could mask a later route() conflict. The side-channel remains for the chaining variants. - Document that router registration is not thread-safe. - Tests: regression coverage for glz::expected data members (REPE + JSON-RPC), a -fno-exceptions test that route() records conflicts on the side-channel, the REST expected<http_error> success path, and updated try_* expectations.
) Adds rest_expected_data_member_is_serialized_not_intercepted: a registered glz::expected data member is read as a stored value (serialized JSON shape, 2xx status), not reinterpreted as an HTTP error. REST never had the #2265 bug because it keeps the stored-value path separate from handler-result writing, but nothing asserted it until now. This completes the cross-protocol matrix, so REPE, JSON-RPC, and REST each cover both the handler-error and the stored-data rows. Verified PASSED in both the exceptions and -fno-exceptions builds.
…#2265) Design note capturing the reasoning behind the #2265 fix and a layered plan to structurally prevent the regression from recurring: one writer vocabulary across REPE/JSON-RPC/REST, a protocol_writer concept, and an optional typed-intent wrapper. Includes performance/compile-time analysis (the seam is a compile-time dispatch change with no hot-path cost) and a cross-protocol test matrix. The seam itself is a future, self-contained follow-up, not part of this PR; the doc is the durable record of the decision.
Returning a full-fidelity JSON-RPC error from a handler required spelling out
glz::unexpected(glz::rpc::error{glz::rpc::error_e::..., std::optional<std::string>{...}})
and repeating the expected<T, glz::rpc::error> return type. Add a thin
convenience layer in glz::rpc:
- result<T>: alias for glz::expected<T, glz::rpc::error>, so the type is not
spelled twice (stored signature + lambda trailing return).
- fail(code, data) and the named helpers invalid_params/invalid_request/
method_not_found/internal_error/parse_error: each returns
glz::unexpected<error> carrying that code, with the optional argument placed
in the JSON-RPC `data` member while `message` stays the standard code text.
The error struct, write_response/set_handler_error translation, and wire output
are unchanged; these are pure additions. The std::optional<std::string> data
parameter matches the owned destination field exactly (a string_view would
allocate anyway and could not distinguish absent from empty), and direct
rpc::error{...} construction remains for setting a custom message or a
server-error code.
Wire the helpers through the docs/examples and drop the now-unnecessary explicit
std::optional<std::string>{...} wrapper (the string literal converts implicitly).
The error_ctx and bare-string shortcuts in set_handler_error placed the handler's detail text in "message" and emitted no "data", diverging from every other internal-error site (the thrown path, serialize-failure, and the REPE bridge), which keep the reserved -32603 code's standard "Internal error" message and carry detail in "data". Since the handler supplies no JSON-RPC code in these cases, the reserved code's standard message is the spec-faithful choice and the custom text is "additional information" -> data. Empty detail now emits no data member. This makes a handler that switches between throwing and returning glz::unexpected produce byte-identical wire output, the equivalence #2265 was built to provide. Tightened the two JSONRPC assertions to pin message and data position so the convention cannot silently drift back.
…exceptions on The *_noexceptions test targets only disable exceptions on GCC/Clang, where glz_test_common adds -fno-exceptions. Its MSVC branch sets /GR- (RTTI off) but no exception-disabling flag, and CMake injects /EHsc by default, so on Windows glz_test_common is identical to glz_test_exceptions: the targets are exceptions- enabled duplicates and the library's !__cpp_exceptions paths go uncompiled there, despite both Windows CI jobs running ctest. This is a pre-existing, suite-wide condition (glz_test_common backs ~42 test dirs), not introduced here, but this PR added the first explicitly named _noexceptions targets and a comment asserting -fno-exceptions unconditionally. Make the harness honest: - glz_test_common MSVC branch: document that exceptions stay enabled and why a real fix (_HAS_EXCEPTIONS=0 + stripping /EHsc) is its own effort. - registry_no_exceptions_test: scope the -fno-exceptions claim to GCC/Clang and note the MSVC variant mirrors the exceptions build. Proper MSVC no-exceptions coverage is left as a follow-up. Comment-only; no build behavior changes.
REST's registry_impl has no register_merge_endpoint (only REPE and
JSON-RPC do), so on(glz::merge<Ts...>&) and the new try_on(glz::merge<Ts...>&)
were non-instantiable under REST: a hard error deep in the implementation
(registry.hpp register_merge_endpoint lookup) rather than at the call site.
The code was dead rather than broken (no caller pairs REST with merge), but
the API advertised a REST merge capability that does not exist.
Add `&& Proto != REST` to the requires-clause on both overloads so a REST
caller gets an honest "no matching function / constraints not satisfied"
diagnostic. With try_on(merge) now non-REST only, its REST route-error branch
is unreachable and is removed, leaving the REPE/JSON-RPC path (registration
cannot fail) of `on(merged); return {};`.
Add a compile-time guard (static_asserts in registry_no_exceptions_test) that
locks the contract in: REST must not expose on/try_on(merge); REPE and JSON-RPC
must. The asserts fail to compile if the constraint is dropped, and flag the
case if REST merge is ever implemented. Implementing a REST combined-view root
endpoint is a feature, intentionally out of scope here.
Refs #2265
The string-like bullet claimed that returning glz::unexpected(msg) and throwing "produce the same response". Only the error code matches (error_code::parse_error): the thrown path wraps the message via build_registry_error(query, ...) for context (registry call()), while the expected path uses the returned string verbatim, so the response bodies differ. Reword to claim code-equivalence only and note the body difference. The body difference is intentional and left as-is: a thrown exception is an uncontrolled escape that benefits from "which method failed" context, whereas a deliberate glz::unexpected(msg) should carry the handler's exact message. The analogous JSON-RPC comment is accurate (its thrown path uses the raw e.what() with no wrapping, matching the expected path) and is unchanged. Refs #2265
An error state returned by a handler must carry a real error. A degenerate success code would otherwise let a failure masquerade as a success (or redirect) on the wire: - REPE: error_code::none -> parse_error - JSON-RPC: error_e::no_error (which would emit "code":0) -> internal (-32603), preserving custom messages and data - REST: glz::http_error with status < 400 -> 500, message preserved Adds regression tests covering each path.
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
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.
Closes #2265.
The RPC
glz::registry(REPE, JSON-RPC, and REST) andglz::http_routercould not compile under-fno-exceptions, and route-registration conflicts were silently swallowed tostderr. This PR makes both usable without exceptions and routes every error through an explicit, programmatic channel rather than hiding it. Nothing is swallowed to stderr, turned into UB, orabort()ed.Design principle: surface errors, never hide them
Glaze's own (de)serialization already reports via error codes (
glz::error_ctx/glz::expected), not exceptions. The only throws in this path came from (a)http_routerroute conflicts and (b) the registry'stry/catcharound user handler code. Each is given a non-throwing channel; exceptions remain a convenience layer on top, never the only way to learn about a failure.http_routeradd_route/addnow returnglz::expected<void, std::string>instead of throwing on a structural conflict (mismatched:param/*wildcardnames at a position, or a wildcard that isn't the final segment). Re-registering an identical route still overwrites, as before.route()/stream()/websocket()throw on a conflict when exceptions are enabled (restoring the documented@throwscontract that the previous stderr-swallow silently violated) and record a queryable error under-fno-exceptions.try_route()plushas_route_error()/route_error()/clear_route_error().glz::registrytry/catchsites are guarded by#if __cpp_exceptions. They only ever guarded user-thrown exceptions, so eliding them under-fno-exceptionsis sound.try_on()(object andglz::mergeoverloads) returningglz::expected<void, std::string>.Failing a request without throwing
A handler may now fail a request by returning
glz::expected<T, E>; translation is centralized in each protocol'swrite_response(no per-wrapper changes). This directly resolves the second half of #2265 ("std::expected to jsonrpc error translation"):glz::rpc::error(full fidelity),rpc::error_e,error_ctx, or any string-like (→ internal error-32603).glz::error_code,error_ctx, or any string-like (→error_code::parse_error, the same code produced when a thrown handler is caught, so throwing and returning an error are equivalent).Ergonomic handler errors:
glz::rpc::resultand factory helpersThe full-fidelity error return above is correct but verbose: the return type is written twice and the error is constructed by hand. Thin additions in
glz::rpccollapse it, with no change to behavior or wire output:result<T>— alias forglz::expected<T, glz::rpc::error>, so the type is not spelled twice (the stored handler signature and the lambda's trailing return type).fail(code, data)plus the named helpersinvalid_params/invalid_request/method_not_found/internal_error/parse_error— each returnsglz::unexpected<error>for that code. The optional argument is carried in the JSON-RPCdatamember, whilemessagestays the standard text for the code.This emits byte-for-byte the same response as the longhand above. These are pure additions: the
errortype and theset_handler_error/write_responsetranslation are untouched, and constructingglz::rpc::error{...}directly remains the path for setting a custommessageor an implementation-defined server-error code. Thedataparameter isstd::optional<std::string>— the owned destination type, since astd::string_viewwould allocate anyway and could not distinguish absent from empty. The docs and thejson-rpcexample are updated to this form, dropping the now-unnecessary explicitstd::optional<std::string>{...}wrapper (a string literal converts implicitly).Testing
registry_no_exceptions_testis compiled twice (exceptions and-fno-exceptions, viaglaze_BUILD_REGISTRY_NOEXCEPT_TESTS) and asserts identical behavior: router conflict surfacing, REPE/JSON-RPCexpectedfailure translation, and RESTtry_on.repe_test,jsonrpc_registry_test, andregistry_view_testnow also build_noexceptionsvariants; their exceptions-only throwing tests are guarded.-Werror, clang) and the registry/router/rpc ctests pass in both configurations.Compatibility
route_error()/try_*(no-exceptions). No existing test registered a conflicting route.This is a from-scratch take on the problem explored in #2564 (compile-only guards) and #2328 (non-throwing registration API); it combines the compile fix with explicit error reporting and adds the handler-failure path neither covered.