Skip to content

Commit 9b7b732

Browse files
etrclaude
andcommitted
TASK-043: Doxygen / inline doc refresh
Refresh public-header doxygen blocks to match the v2.0 surface, and add a make-check gate (check-doxygen.sh) that fails on any substantive doxygen warning. The gate is wired into check-local via Makefile.am following the precedent established by TASK-040/041/042. Substantive doxygen-source warnings fixed (17 → 0): * `webserver.hpp` register_path/register_prefix/register_ws_resource: the shared_ptr overloads used `@copydoc <template-overload>` followed by an `@param res` override, which doxygen flags as "too many @param". Replace the @copydoc with a fresh self-contained block on each overload (same prose, no duplicate @param). * `http_request.hpp` check_digest_auth / check_digest_auth_digest: add explicit @param blocks for realm/password/nonce_timeout/max_nc/algo on check_digest_auth, and replace the @copydoc-based block on check_digest_auth_digest with a self-contained one (copydoc was inheriting `password` which is not in the digest variant's signature). * `http_request.hpp` get_or_create_file_info: add missing `@param key`. * `http_resource.hpp` render: name the previously-anonymous `req` parameter so the `@param req` in the doc block resolves. * `http_utils.hpp` get_ip_str: drop the stale `@param maxlen` block left over from the v1 two-arg signature. * `http_utils.hpp` header_comparator::operator() and arg_comparator:: operator(): rename `@param first/second` to match the actual `x, y` signature, and document the case-sensitivity contract on the arg variant. Acceptance-criterion content (per the task action items): * AC bullet 3 — threading: add a `### Threading contract (DR-008 / §5.1)` block on the `webserver` class summarising the 5-point contract (re-entrant registration; stop/destructor must not run on a handler thread; per-request http_request; exclusive http_response; concurrent user callbacks). * AC bullet 4 — error propagation: add a load-bearing block on `create_webserver::internal_error_handler` that spells out the DR-009 §5.2 6-point contract verbatim, cross-linking to @ref webserver, not_found_handler, method_not_allowed_handler, feature_unavailable. Also document `internal_error_handler_t` (the typedef) explaining the std::string_view ownership rules. * AC bullet 5 — feature_unavailable throw sites: enumerate the throw sites on the feature_unavailable class itself (webserver ctor for TLS/BAUTH/DAUTH, register_ws_resource / unregister_ws_resource for HAVE_WEBSOCKET, and every send_* / close on websocket_session). * AC bullet 2 — cross-links: `@see` pairs added between block_ip ↔ unblock_ip, register_path ↔ register_prefix, the unregister_* family, register_ws_resource ↔ unregister_ws_resource, and the three error-handler setters. * Class-level docs added on `create_webserver` (fluent-builder contract, eager validation, explicit webserver ctor), and on `websocket_session` / `websocket_handler` (per-method param/return/ throws blocks including the HAVE_WEBSOCKET-off feature_unavailable behaviour). Verification: * `make check` green (48 testsuite entries pass; check-headers, check-examples, check-readme, check-release-notes, check-doxygen, check-install-layout, check-hygiene all PASS). * `make doxygen-run` now produces zero substantive warnings; the remaining stderr lines (obsolete config tags, dot/graphviz env artifacts when graphviz is missing) are explicitly filtered by scripts/check-doxygen.sh. * Spot-check on 11 v2.0-renamed/reshaped public methods (register_path, register_prefix, internal_error_handler, block_ip, unblock_ip, route, on_get, use_ssl, basic_auth, register_ws_resource, feature_unavailable) — each has a current `///` or `/**` block with @param/@return/@see reflecting the v2.0 signature. The gate skips with exit 0 when doxygen is not installed (mirrors the tolerance pattern used by the other check-* scripts); in CI the gate runs whenever the doxygen package is present. PRD §2 documentation NFR; §13 documentation deliverable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1da2ee1 commit 9b7b732

9 files changed

Lines changed: 516 additions & 36 deletions

File tree

Makefile.am

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ endif
4040

4141
EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh scripts/validate-version.sh \
4242
scripts/check-examples.sh scripts/check-readme.sh scripts/check-release-notes.sh \
43+
scripts/check-doxygen.sh \
4344
scripts/verify-installed-examples.sh \
4445
test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_umbrella.cpp \
4546
test/headers/consumer_post_umbrella.cpp \
@@ -283,7 +284,7 @@ check-hygiene:
283284
# shared staged install to avoid paying two full `make install` costs on
284285
# every `make check`. Both sub-checks can still be invoked standalone (they
285286
# will do their own install when CHECK_*_SHARED is not set).
286-
check-local: check-headers check-examples check-readme check-release-notes
287+
check-local: check-headers check-examples check-readme check-release-notes check-doxygen
287288
@echo "=== Shared staged install for check-install-layout and check-hygiene ==="
288289
@rm -rf $(abs_top_builddir)/.shared-check-stage
289290
@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(abs_top_builddir)/.shared-check-stage >check-shared-install.log 2>&1 || { \
@@ -319,7 +320,15 @@ check-release-notes:
319320
@echo "=== check-release-notes: enforce TASK-042 invariants on RELEASE_NOTES.md ==="
320321
@$(top_srcdir)/scripts/check-release-notes.sh
321322

322-
.PHONY: check-headers check-install-layout check-hygiene check-examples check-readme check-release-notes
323+
# TASK-043: run doxygen and fail on any substantive warning. The script
324+
# delegates to `make doxygen-run` and parses stderr; environmental noise
325+
# (obsolete config tags, missing graphviz) is filtered out. Skips with
326+
# exit 0 if doxygen is not installed.
327+
check-doxygen:
328+
@echo "=== check-doxygen: enforce TASK-043 zero-warning invariant ==="
329+
@BUILD_DIR="$(abs_top_builddir)" $(top_srcdir)/scripts/check-doxygen.sh
330+
331+
.PHONY: check-headers check-install-layout check-hygiene check-examples check-readme check-release-notes check-doxygen
323332

324333
# TASK-039: top-level convenience rule that descends into test/ to
325334
# build and run the bench binaries (see test/Makefile.am and

scripts/check-doxygen.sh

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
#!/usr/bin/env bash
2+
#
3+
# check-doxygen.sh — enforce TASK-043 invariant on the doxygen build.
4+
#
5+
# Runs `make doxygen-run` in the active build tree and asserts that the
6+
# output contains zero substantive warnings or errors. Substantive means:
7+
# anything from the doxygen parser about the SOURCE under
8+
# `src/httpserver/` — e.g. an undocumented @param, a mismatched parameter
9+
# name, a stale @copydoc target.
10+
#
11+
# Lines we deliberately filter OUT (environmental, not doc-content issues):
12+
#
13+
# E1. "Tag '<NAME>' at line N of file '...doxyconfig.in' has become
14+
# obsolete." — older config tags carried over from doxywizard.
15+
# E2. "doxygen no longer ships with the FreeSans font." — packaging.
16+
# E3. "the dot tool could not be found ..." — graphviz absent locally.
17+
# E4. "Failed to rename ... .dot.png to ... .png" — dot post-processing
18+
# failure (typically dot absent or installed late).
19+
# E5. "Problems running dot: exit code=..." — same root cause as E4.
20+
# E6. "Caller graph for '<sym>' not generated, too many nodes (N),
21+
# threshold is M." — informational, DOT_GRAPH_MAX_NODES threshold.
22+
#
23+
# Everything else under the keywords `warning:` or `error:` is a real
24+
# doc issue and FAILS the gate. Exits non-zero on the first violation.
25+
#
26+
# Behaviour when doxygen is not installed: SKIP (exit 0). The gate is
27+
# CI-runnable on developer machines without doxygen; CI is expected to
28+
# install it. This mirrors how scripts/check-readme.sh and friends are
29+
# tolerant of missing tools.
30+
31+
set -euo pipefail
32+
33+
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
34+
35+
# Locate the build tree. Prefer $BUILD_DIR, else REPO_ROOT/build, else
36+
# fall back to the current working directory (assumes invoker is already
37+
# inside the build dir, mirroring `make` behaviour).
38+
BUILD_DIR="${BUILD_DIR:-}"
39+
if [ -z "$BUILD_DIR" ]; then
40+
if [ -d "$REPO_ROOT/build" ] && [ -f "$REPO_ROOT/build/Makefile" ]; then
41+
BUILD_DIR="$REPO_ROOT/build"
42+
elif [ -f "Makefile" ]; then
43+
BUILD_DIR="$(pwd)"
44+
else
45+
echo "check-doxygen: FAIL: no build directory found (set BUILD_DIR)" >&2
46+
exit 1
47+
fi
48+
fi
49+
50+
if ! command -v doxygen >/dev/null 2>&1; then
51+
echo "check-doxygen: SKIP — doxygen not installed (gate is enforced in CI)"
52+
exit 0
53+
fi
54+
55+
LOGFILE="$(mktemp -t check-doxygen.XXXXXX)"
56+
trap 'rm -f "$LOGFILE"' EXIT
57+
58+
echo "check-doxygen: invoking 'make doxygen-run' in $BUILD_DIR"
59+
# Force a fresh doxygen invocation. The make rule's recipe already does
60+
# `rm -rf doxygen-doc` but only fires when an input is newer than the
61+
# tag file. When NO header has changed (e.g. CI rerun) doxygen does not
62+
# re-execute and warnings from a prior good run would not reappear here.
63+
# Removing the tag file forces the rule to fire every time.
64+
rm -f "$BUILD_DIR/doxygen-doc/libhttpserver.tag"
65+
if ! ( cd "$BUILD_DIR" && make doxygen-run ) >"$LOGFILE" 2>&1; then
66+
echo "check-doxygen: FAIL: 'make doxygen-run' exited non-zero" >&2
67+
sed -n '1,200p' "$LOGFILE" >&2
68+
exit 1
69+
fi
70+
71+
# Strip the noisy-but-harmless environmental lines. The grep is portable
72+
# (BSD/GNU): -E for ERE alternation, single anchored pattern per line.
73+
FILTER='Tag .* has become obsolete\.'
74+
FILTER+='|doxygen no longer ships with the FreeSans font'
75+
FILTER+='|the dot tool could not be found'
76+
FILTER+='|Failed to rename .*\.dot\.png to'
77+
FILTER+='|Problems running dot: exit code='
78+
FILTER+='|Caller graph for .* not generated, too many nodes'
79+
80+
REAL_WARNINGS="$(grep -E '(^|: )(warning|error):' "$LOGFILE" | grep -Ev "$FILTER" || true)"
81+
82+
if [ -n "$REAL_WARNINGS" ]; then
83+
echo "check-doxygen: FAIL: substantive doxygen warnings/errors found:" >&2
84+
echo "$REAL_WARNINGS" >&2
85+
echo "" >&2
86+
echo "(Full log: $LOGFILE — copy aside if needed before this script exits.)" >&2
87+
# Preserve log on failure for inspection.
88+
trap - EXIT
89+
exit 1
90+
fi
91+
92+
echo "check-doxygen: PASS — doxygen-run produced zero substantive warnings"

src/httpserver/create_webserver.hpp

Lines changed: 118 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,25 @@ namespace httpserver {
4646
class webserver;
4747
class http_request;
4848

49-
// TASK-030 / PRD-NAM-REQ-003 — see specs/architecture/04-components/create-webserver.md.
49+
/**
50+
* Callback signature for the 404 and 405 error-page handlers.
51+
*
52+
* Returned by value; the webserver writes the response to the wire.
53+
* (TASK-030 / PRD-NAM-REQ-003 — see specs/architecture/04-components/create-webserver.md.)
54+
*/
5055
typedef std::function<http_response(const http_request&)> error_handler;
5156

52-
// TASK-031 / DR-009 §5.2 — internal_error_handler receives the originating
53-
// exception's message as a non-owning view; see create-webserver.md.
57+
/**
58+
* Callback signature for @ref create_webserver::internal_error_handler.
59+
*
60+
* The handler receives the originating exception's message as a non-owning
61+
* `std::string_view` — for `std::exception` derivatives this is `e.what()`;
62+
* for non-`std::exception` throws (`throw 42`) it is the literal string
63+
* `"unknown exception"`. The view is valid only for the duration of the
64+
* call; copy if you need to retain it.
65+
*
66+
* Full contract: DR-009 §5.2 (see @ref webserver class-level block).
67+
*/
5468
typedef std::function<http_response(const http_request&, std::string_view message)> internal_error_handler_t;
5569

5670
typedef std::function<bool(const std::string&)> validator_ptr;
@@ -66,6 +80,25 @@ namespace http { class file_info; }
6680
typedef std::function<bool(const std::string&, const std::string&, const http::file_info&)> file_cleanup_callback_ptr;
6781
typedef std::function<std::shared_ptr<http_response>(const http_request&)> auth_handler_ptr;
6882

83+
/**
84+
* Fluent builder for @ref webserver instances (PRD-NAM-REQ-004,
85+
* PRD-CFG-REQ-001..003 / TASK-033).
86+
*
87+
* Each setter returns `*this` so calls can chain:
88+
* `webserver ws{create_webserver{}.port(8080).use_ssl(true).max_threads(4)};`
89+
*
90+
* Setters validate eagerly where the input domain is well-defined
91+
* (e.g. @ref port rejects values outside `[0, 65535]`, all `int` setters
92+
* reject negatives); feature-gated settings (@ref use_ssl,
93+
* @ref basic_auth, @ref digest_auth, websocket registration) are
94+
* validated when the @ref webserver constructor consumes the builder,
95+
* not at the setter — so a builder configured for an unsupported
96+
* feature throws @ref feature_unavailable from `webserver(create_webserver)`
97+
* rather than from the setter.
98+
*
99+
* The @ref webserver constructor is `explicit`: callers must direct-init
100+
* (`webserver ws{cw};`) rather than rely on implicit conversion.
101+
*/
69102
class create_webserver {
70103
public:
71104
create_webserver() = default;
@@ -102,6 +135,16 @@ class create_webserver {
102135
create_webserver& max_thread_stack_size(int v) { check_non_negative("max_thread_stack_size", v); _max_thread_stack_size = v; return *this; }
103136

104137
// Boolean flag setters (TASK-033 / PRD-CFG-REQ-001).
138+
/**
139+
* Enable TLS for the webserver (HTTPS).
140+
*
141+
* On a `HAVE_GNUTLS`-off build, constructing a @ref webserver from a
142+
* builder with `use_ssl(true)` throws @ref feature_unavailable.
143+
* Defaults to `false`.
144+
*
145+
* @param enable `true` to enable TLS, `false` to disable.
146+
* @return reference to this builder for chaining.
147+
*/
105148
create_webserver& use_ssl(bool enable = true) { _use_ssl = enable; return *this; }
106149
create_webserver& use_ipv6(bool enable = true) { _use_ipv6 = enable; return *this; }
107150
create_webserver& use_dual_stack(bool enable = true) { _use_dual_stack = enable; return *this; }
@@ -125,7 +168,27 @@ class create_webserver {
125168
// validation lives in webserver(const create_webserver&), which
126169
// throws feature_unavailable when this is set to true on a
127170
// HAVE_BAUTH-off build.
171+
/**
172+
* Enable HTTP Basic authentication on the webserver.
173+
*
174+
* On a `HAVE_BAUTH`-off build, constructing a @ref webserver from a
175+
* builder with `basic_auth(true)` throws @ref feature_unavailable.
176+
* Default value depends on the build flag (see TASK-034).
177+
*
178+
* @param enable `true` to enable Basic auth, `false` to disable.
179+
* @return reference to this builder for chaining.
180+
*/
128181
create_webserver& basic_auth(bool enable = true) { _basic_auth_enabled = enable; return *this; }
182+
/**
183+
* Enable HTTP Digest authentication on the webserver.
184+
*
185+
* On a `HAVE_DAUTH`-off build, constructing a @ref webserver from a
186+
* builder with `digest_auth(true)` throws @ref feature_unavailable.
187+
* Default value depends on the build flag.
188+
*
189+
* @param enable `true` to enable Digest auth, `false` to disable.
190+
* @return reference to this builder for chaining.
191+
*/
129192
create_webserver& digest_auth(bool enable = true) { _digest_auth_enabled = enable; return *this; }
130193
create_webserver& deferred(bool enable = true) { _deferred_enabled = enable; return *this; }
131194
create_webserver& regex_checking(bool enable = true) { _regex_checking = enable; return *this; }
@@ -141,8 +204,60 @@ class create_webserver {
141204
create_webserver& generate_random_filename_on_upload(bool enable = true) { _generate_random_filename_on_upload = enable; return *this; }
142205
create_webserver& single_resource(bool enable = true) { _single_resource = enable; return *this; }
143206
create_webserver& tcp_nodelay(bool enable = true) { _tcp_nodelay = enable; return *this; }
207+
/**
208+
* Install a handler invoked when no resource matches the request path (HTTP 404).
209+
*
210+
* The handler returns an @ref http_response by value; its status
211+
* code, headers, and body are sent on the wire as-is. If null, a
212+
* default 404 response is generated.
213+
*
214+
* @param h error_handler callback; pass `nullptr` to clear.
215+
* @return reference to this builder for chaining.
216+
* @see method_not_allowed_handler, internal_error_handler
217+
*/
144218
create_webserver& not_found_handler(error_handler h) { _not_found_handler = std::move(h); return *this; }
219+
/**
220+
* Install a handler invoked when a resource matches the path but
221+
* not the HTTP method (HTTP 405).
222+
*
223+
* The handler returns an @ref http_response by value. If null, a
224+
* default 405 response is generated.
225+
*
226+
* @param h error_handler callback; pass `nullptr` to clear.
227+
* @return reference to this builder for chaining.
228+
* @see not_found_handler, internal_error_handler
229+
*/
145230
create_webserver& method_not_allowed_handler(error_handler h) { _method_not_allowed_handler = std::move(h); return *this; }
231+
/**
232+
* Install the handler invoked when a registered request handler
233+
* throws (HTTP 500 by default).
234+
*
235+
* This is the load-bearing extension point for the dispatch
236+
* error-propagation contract (DR-009 §5.2 / PRD-FLG-REQ-002). The
237+
* contract — full statement on the @ref webserver class block — is:
238+
*
239+
* 1. The handler call is wrapped in `try/catch (std::exception&) / catch (...)`.
240+
* 2. On `std::exception`: the message is logged via @ref log_error,
241+
* then @p h is invoked with `e.what()` as the @ref internal_error_handler_t
242+
* `message` argument.
243+
* 3. On non-`std::exception`: same path with the message replaced
244+
* by the literal string `"unknown exception"`.
245+
* 4. If @p h itself throws while servicing the above, the failure
246+
* is logged generically and a hardcoded 500 with an EMPTY body
247+
* is sent. No exception ever escapes into libmicrohttpd.
248+
* 5. @ref feature_unavailable (a `std::runtime_error` subclass) is
249+
* NOT mapped specially: it lands as a generic 500 like any
250+
* other `std::exception`.
251+
* 6. May run concurrently from multiple MHD worker threads;
252+
* implementations MUST be thread-safe.
253+
*
254+
* If @p h is null, the default response is a 500 with the message
255+
* in the body (see DR-009).
256+
*
257+
* @param h @ref internal_error_handler_t callback; pass `nullptr` to clear.
258+
* @return reference to this builder for chaining.
259+
* @see webserver, not_found_handler, method_not_allowed_handler, feature_unavailable
260+
*/
146261
create_webserver& internal_error_handler(internal_error_handler_t h) { _internal_error_handler = std::move(h); return *this; }
147262
create_webserver& file_cleanup_callback(file_cleanup_callback_ptr v) { _file_cleanup_callback = v; return *this; }
148263
create_webserver& auth_handler(auth_handler_ptr v) { _auth_handler = v; return *this; }

src/httpserver/feature_unavailable.hpp

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,60 @@
3131

3232
namespace httpserver {
3333

34-
// Exception thrown when a build-time-disabled feature is invoked at runtime.
35-
// The class is unconditionally available regardless of HAVE_* flags so that
36-
// downstream code can always write
37-
// try { ... } catch (const httpserver::feature_unavailable&) { ... }
38-
// even in builds that compiled out the optional feature in question.
39-
//
40-
// The class is header-only (and inline) on purpose: it has no library
41-
// dependencies, must be cheap to throw from anywhere in the codebase, and
42-
// avoids ABI churn for what is effectively a labelled std::runtime_error.
34+
/**
35+
* Exception thrown when a build-time-disabled feature is invoked at runtime.
36+
*
37+
* The class is unconditionally available regardless of `HAVE_*` flags so
38+
* that downstream code can always write
39+
* @code
40+
* try { ... } catch (const httpserver::feature_unavailable&) { ... }
41+
* @endcode
42+
* even in builds that compiled out the optional feature in question.
43+
*
44+
* The class is header-only (and inline) on purpose: it has no library
45+
* dependencies, must be cheap to throw from anywhere in the codebase, and
46+
* avoids ABI churn for what is effectively a labelled `std::runtime_error`.
47+
*
48+
* ### Throw sites (architecture spec §7)
49+
*
50+
* The library throws `feature_unavailable` from these sites; each one
51+
* pairs the feature label with the `HAVE_*` build flag that gates it:
52+
*
53+
* - @ref webserver::webserver — when constructing from a builder that
54+
* enables `use_ssl(true)` on a `HAVE_GNUTLS`-off build, or
55+
* `basic_auth(true)` on a `HAVE_BAUTH`-off build, or
56+
* `digest_auth(true)` on a `HAVE_DAUTH`-off build.
57+
* - @ref webserver::register_ws_resource and
58+
* @ref webserver::unregister_ws_resource — on a `HAVE_WEBSOCKET`-off
59+
* build (every websocket entry point throws this).
60+
* - @ref websocket_session::send_text, @ref websocket_session::send_binary,
61+
* @ref websocket_session::send_ping, @ref websocket_session::send_pong,
62+
* and @ref websocket_session::close — on a `HAVE_WEBSOCKET`-off build,
63+
* so that downstream handlers that capture a session reference get a
64+
* uniform exception type rather than a link-time error.
65+
*
66+
* Catching `feature_unavailable` (or its base `std::runtime_error`) on
67+
* the dispatch path is handled by the standard
68+
* @ref webserver internal_error_handler contract — it surfaces as a
69+
* generic 500 unless the application installs an
70+
* @ref create_webserver::internal_error_handler that special-cases it.
71+
*
72+
* @see create_webserver::use_ssl, create_webserver::basic_auth,
73+
* create_webserver::digest_auth, webserver::register_ws_resource,
74+
* websocket_session
75+
*/
4376
class feature_unavailable : public std::runtime_error {
4477
public:
78+
/**
79+
* Construct with the feature name and the build flag that gates it.
80+
*
81+
* The resulting `what()` is
82+
* `"feature '<feature>' unavailable: built without <build_flag>"`.
83+
*
84+
* @param feature human-readable feature label (e.g. `"TLS"`, `"WebSocket"`).
85+
* @param build_flag the autoconf-defined `HAVE_*` flag that was off
86+
* in this build (e.g. `"HAVE_GNUTLS"`).
87+
*/
4588
feature_unavailable(std::string_view feature, std::string_view build_flag)
4689
: std::runtime_error(compose_message(feature, build_flag)) {}
4790

0 commit comments

Comments
 (0)