Skip to content

Commit 83eb784

Browse files
etrclaude
andcommitted
TASK-049: fire handler_exception + wire internal_error_handler alias
Convert the DR-009 §5.2 dispatch-exception path into a hookable chain. User-added handler_exception hooks run first in registration order; the v1 internal_error_handler becomes a last-position alias slot. The first hook to return respond_with() wins; a throwing hook is caught and the chain continues (DR-012 §4.10 - the one phase where exception-in-hook does NOT abort, because the chain itself IS exception recovery). When every hook and the alias either throw or pass(), the dispatcher emits the hardcoded empty-body 500 directly without re-invoking the alias. Implementation: - webserver_impl gains handler_exception_alias_, a dedicated single-slot std::function written once at construction (never in the user vector, so its last-position ordering is structural). - fire_handler_exception in hook_handle.cpp snapshots the user vector under shared_lock, iterates with per-hook try/catch, then invokes the alias slot. - dispatch_resource_handler's two catch arms now route through handle_dispatch_exception() which takes the chain path when any user hook is registered OR the alias is wired, and falls back to the v1 run_internal_error_handler_safely path otherwise. - install_default_alias_hooks_ writes the alias slot via the extracted install_internal_error_alias_ helper (keeps host CCN at baseline). - Doxygen on create_webserver::internal_error_handler and the class- level error-propagation contract on webserver document the alias and reference DR-012. Tests (all 68 pass sequentially): - hooks_handler_exception_chain: A=pass, B=respond_with(418), alias C never invoked. - hooks_handler_exception_user_handler_throws_continues_chain: A throws; chain continues to B; "alpha" surfaced in log_error. - hooks_handler_exception_fallback_to_hardcoded_500: A=pass, B throws, alias C throws -> empty-body 500; alias C called exactly once (pins no-re-entry contract). - hooks_handler_exception_slot: unit pin that internal_error_handler populates the dedicated last-position slot and leaves the user vector at size 0. - hooks_no_firing: handler_exception added to the not_yet_wired exclude list with explanatory comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 67dd238 commit 83eb784

13 files changed

Lines changed: 791 additions & 16 deletions

src/detail/webserver_aliases.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,37 @@ namespace httpserver {
7676

7777
namespace {
7878

79+
// TASK-049: build the std::function stored in
80+
// webserver_impl::handler_exception_alias_ from a (non-null) user-supplied
81+
// internal_error_handler callable. Extracted from
82+
// install_default_alias_hooks_ to keep that function under the CCN bar.
83+
std::function<hook_action(const handler_exception_ctx&)>
84+
make_internal_error_alias_(internal_error_handler_t user_handler) {
85+
return [user_handler = std::move(user_handler)](
86+
const handler_exception_ctx& ctx) -> hook_action {
87+
if (ctx.request == nullptr) {
88+
// Defensive: caller always passes a non-null request, but
89+
// stay benign if a future call site changes that contract.
90+
return hook_action::pass();
91+
}
92+
return hook_action::respond_with(
93+
user_handler(*ctx.request, ctx.message));
94+
};
95+
}
96+
97+
// TASK-049: install the internal_error_handler alias into the dedicated
98+
// last-position slot on webserver_impl. Extracted from
99+
// install_default_alias_hooks_ so the added `if` does not push the host
100+
// function over the CCN bar. See webserver_impl::handler_exception_alias_
101+
// for the lifetime contract (write-once-at-construction).
102+
void install_internal_error_alias_(
103+
detail::webserver_impl* impl,
104+
internal_error_handler_t user_handler) {
105+
if (user_handler == nullptr) return;
106+
impl->handler_exception_alias_ =
107+
make_internal_error_alias_(std::move(user_handler));
108+
}
109+
79110
// Serialize an allowed-method set into the comma-separated value
80111
// expected by the HTTP `Allow:` header. Enum-declaration order:
81112
// GET, HEAD, POST, PUT, DELETE, CONNECT, OPTIONS, TRACE, PATCH.
@@ -204,6 +235,35 @@ void webserver::install_default_alias_hooks_() {
204235
})))
205236
.detach();
206237
}
238+
239+
// ----------------------------------------------------------------
240+
// internal_error_handler -> handler_exception alias slot (LAST position).
241+
// [TASK-049]
242+
//
243+
// This is an alias. Calling internal_error_handler(fn) on the builder
244+
// makes the user callable the LAST-position fallback in the
245+
// handler_exception chain (DR-012 §4.10, PRD-HOOK-REQ-009).
246+
//
247+
// Unlike auth_handler / method_not_allowed_handler / not_found_handler
248+
// (which install at the FIRST position via add_hook so they short-
249+
// circuit before user hooks), the internal_error_handler alias must
250+
// fire LAST so user-added handler_exception hooks have a chance to
251+
// recover first. We achieve this by storing the alias in the
252+
// dedicated webserver_impl::handler_exception_alias_ slot rather than
253+
// push_back-ing into the hooks_handler_exception_ vector. The fire
254+
// site (fire_handler_exception in src/hook_handle.cpp) iterates the
255+
// user vector first and only then invokes the alias slot.
256+
//
257+
// The alias body invokes the user-supplied callable with the
258+
// originating exception's message and returns
259+
// hook_action::respond_with(response). If the user callable itself
260+
// throws, fire_handler_exception's catch arm absorbs it and returns
261+
// nullopt; the caller in dispatch_resource_handler then emits the
262+
// hardcoded empty-body 500 DIRECTLY without re-invoking the user
263+
// callable (it has already been seen to throw on this request --
264+
// calling it a second time would observably invoke the user code
265+
// twice for one logical exception). See webserver_dispatch.cpp.
266+
install_internal_error_alias_(impl_.get(), internal_error_handler);
207267
}
208268

209269
} // namespace httpserver

src/detail/webserver_dispatch.cpp

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,49 @@ std::string webserver_impl::serialize_allow_methods(method_set allowed) const {
367367
return header_value;
368368
}
369369

370+
namespace {
371+
372+
// TASK-049: shared body of the two dispatch_resource_handler catch arms.
373+
// Either routes the dispatch-thrown exception through the
374+
// handler_exception hook chain (when any user hooks are registered or
375+
// the internal_error_handler alias slot is wired) or falls back to the
376+
// v1 run_internal_error_handler_safely path. Extracted to keep
377+
// dispatch_resource_handler under the per-function CCN bar.
378+
void handle_dispatch_exception(
379+
webserver_impl* impl,
380+
detail::modded_request* mr,
381+
std::string_view message) {
382+
if (impl->any_hooks_[static_cast<std::size_t>(
383+
hook_phase::handler_exception)]
384+
.load(std::memory_order_relaxed) ||
385+
impl->handler_exception_alias_) {
386+
handler_exception_ctx ctx{
387+
/*request=*/mr->dhr.get(),
388+
/*exception=*/std::current_exception(),
389+
/*message=*/message};
390+
if (auto sc = impl->fire_handler_exception(ctx)) {
391+
mr->response_.emplace(std::move(*sc));
392+
return;
393+
}
394+
// DR-009 §5.2 point 4 extended: every hook (and the alias) ran
395+
// without producing a response -- emit the hardcoded empty-body
396+
// 500 directly. Do NOT re-enter run_internal_error_handler_safely
397+
// here; the alias slot has already invoked the user callable on
398+
// this request.
399+
mr->response_.emplace(
400+
impl->internal_error_page(mr, "", /*force_our=*/true));
401+
return;
402+
}
403+
// Backwards-compat fast path: no handler_exception hooks AND no
404+
// alias wired (the v1 builder did not call internal_error_handler).
405+
// Use the existing safe-call site so the unset-alias default body
406+
// (which surfaces the message) still applies.
407+
mr->response_.emplace(
408+
impl->run_internal_error_handler_safely(mr, message));
409+
}
410+
411+
} // namespace
412+
370413
void webserver_impl::dispatch_resource_handler(detail::modded_request* mr,
371414
const std::shared_ptr<http_resource>& hrm) {
372415
try {
@@ -412,17 +455,18 @@ void webserver_impl::dispatch_resource_handler(detail::modded_request* mr,
412455
}
413456
} catch (const std::exception& e) {
414457
// TASK-031 / DR-009 §5.2 point 2: handler threw std::exception.
415-
// Log via error_logger, forward e.what() to internal_error_handler.
416-
// run_internal_error_handler_safely contains a possible re-throw
417-
// from the user handler (point 4).
458+
// TASK-049 routes the exception through the handler_exception
459+
// hook chain (with the internal_error_handler alias as the
460+
// last-position fallback) inside handle_dispatch_exception.
418461
log_dispatch_error(std::string("dispatch: handler threw "
419462
"std::exception: ") + e.what());
420-
mr->response_.emplace(run_internal_error_handler_safely(mr, e.what()));
463+
handle_dispatch_exception(this, mr, std::string_view{e.what()});
421464
} catch (...) {
422465
// §5.2 point 3: handler threw non-std::exception. Same flow as
423466
// the std::exception arm but with the sentinel message.
424467
log_dispatch_error("dispatch: handler threw unknown exception");
425-
mr->response_.emplace(run_internal_error_handler_safely(mr, "unknown exception"));
468+
handle_dispatch_exception(this, mr,
469+
std::string_view{"unknown exception"});
426470
}
427471
}
428472

src/hook_handle.cpp

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,11 @@ std::string peer_address::to_string() const {
231231
// ws.add_hook() / handle.remove() because we no longer hold the table
232232
// lock by the time the user code runs.
233233

234-
namespace {
235-
236-
constexpr std::size_t kHookSnapshotReserve = 8;
237-
238-
} // namespace
234+
// (Previously an unused kHookSnapshotReserve constant lived here.
235+
// The thread_local snapshot buffers below are now sized lazily on
236+
// first use; the constant was dead code as of TASK-048's perf rework.
237+
// Removed in TASK-049 to silence -Wunused-const-variable under
238+
// --enable-debug.)
239239

240240
// fire_hooks_for_phase: shared dispatch template for all void-returning
241241
// lifecycle hook phases. Snapshots the caller-supplied vector under a
@@ -383,4 +383,72 @@ detail::webserver_impl::fire_before_handler(
383383
this, hooks_before_handler_, ctx, "before_handler");
384384
}
385385

386+
// ---- fire_* (TASK-049) ---------------------------------------------------
387+
//
388+
// handler_exception is the only short-circuit-capable phase whose ctx is
389+
// passed as `const&` (the user cannot mutate the in-flight exception or
390+
// request). It also layers a dedicated single-slot alias on top of the
391+
// user vector (handler_exception_alias_ -- the v1 internal_error_handler
392+
// re-described as a last-position hook). Both of those make the body
393+
// awkward to express via fire_short_circuit_hooks_for_phase<Ctx&>; the
394+
// firing logic is inlined here. The body mirrors the template's
395+
// structure -- snapshot under shared_lock, release, iterate with per-hook
396+
// try/catch -- with an extra tail that invokes the alias slot after the
397+
// user vector is exhausted.
398+
std::optional<::httpserver::http_response>
399+
detail::webserver_impl::fire_handler_exception(
400+
const ::httpserver::handler_exception_ctx& ctx) noexcept {
401+
using EntryVec = std::vector<phase_entry<
402+
::httpserver::hook_action(
403+
const ::httpserver::handler_exception_ctx&)>>;
404+
try {
405+
thread_local EntryVec snapshot;
406+
snapshot.clear();
407+
{
408+
std::shared_lock lock(hook_table_mutex_);
409+
snapshot = hooks_handler_exception_;
410+
}
411+
for (auto& entry : snapshot) {
412+
try {
413+
auto action = entry.fn(ctx);
414+
if (!action.is_pass()) {
415+
return std::move(action).take_response();
416+
}
417+
} catch (const std::exception& e) {
418+
log_dispatch_error(
419+
std::string("hook[handler_exception] threw: ") + e.what());
420+
} catch (...) {
421+
log_dispatch_error(
422+
"hook[handler_exception] threw unknown exception");
423+
}
424+
}
425+
} catch (...) {
426+
log_dispatch_error(
427+
"fire_handler_exception: snapshot copy failed");
428+
}
429+
// Tail: invoke the alias slot, if any. Read without synchronisation;
430+
// the slot is single-writer-at-construction (see webserver_impl.hpp).
431+
//
432+
// Throw containment: a throwing alias is logged with the legacy
433+
// "internal_error_handler threw" prefix so the DR-009 §5.2 point 4
434+
// log contract (and its tests in basic.cpp) is preserved verbatim
435+
// even though the call site has moved from
436+
// run_internal_error_handler_safely into the hook chain.
437+
if (handler_exception_alias_) {
438+
try {
439+
auto action = handler_exception_alias_(ctx);
440+
if (!action.is_pass()) {
441+
return std::move(action).take_response();
442+
}
443+
} catch (const std::exception& e) {
444+
log_dispatch_error(
445+
std::string("internal_error_handler threw: ") + e.what());
446+
} catch (...) {
447+
log_dispatch_error(
448+
"internal_error_handler threw unknown exception");
449+
}
450+
}
451+
return std::nullopt;
452+
}
453+
386454
} // namespace httpserver

src/httpserver/create_webserver.hpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,24 @@ class create_webserver {
268268
* If @p h is null, the default response is a 500 with the message
269269
* in the body (see DR-009).
270270
*
271+
* @note This is an alias. Calling it with a non-null callable
272+
* installs the callable as a LAST-position hook at
273+
* @ref httpserver::hook_phase::handler_exception. Equivalent
274+
* to `ws.add_hook(hook_phase::handler_exception, ...)`
275+
* at webserver construction, except that the alias slot
276+
* ALWAYS fires last in the chain -- user-added
277+
* handler_exception hooks added via `add_hook` run first and
278+
* may short-circuit before the alias is reached. See DR-012
279+
* / §4.10 / PRD-HOOK-REQ-009.
280+
*
281+
* Throwing-hook semantics (per DR-012): a throwing
282+
* handler_exception hook -- user-added or this alias -- is
283+
* caught and the chain CONTINUES to the next hook. If every
284+
* hook (including this alias) either throws or returns
285+
* @ref httpserver::hook_action::pass(), the dispatcher falls
286+
* back to the hardcoded empty-body 500 (DR-009 §5.2 point 4)
287+
* WITHOUT re-invoking @p h.
288+
*
271289
* @param h @ref internal_error_handler_t callback; pass `nullptr` to clear.
272290
* @return reference to this builder for chaining.
273291
* @see webserver, not_found_handler, method_not_allowed_handler, feature_unavailable

src/httpserver/detail/webserver_impl.hpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,24 @@ class webserver_impl {
272272
std::vector<phase_entry<::httpserver::hook_action(
273273
const ::httpserver::handler_exception_ctx&)>>
274274
hooks_handler_exception_;
275+
// TASK-049: internal_error_handler alias slot. Last-position fallback
276+
// in the handler_exception chain. Distinct from hooks_handler_exception_
277+
// because it must fire AFTER all user hooks (DR-012 §4.10) -- the
278+
// opposite of the TASK-048 aliases which run before user hooks. By
279+
// sitting in a dedicated slot rather than the vector, the alias never
280+
// contends for ordering when a user does add_hook(handler_exception, ...).
281+
//
282+
// Lifetime: written exactly once during install_default_alias_hooks_()
283+
// at webserver construction, before start() is called -- the daemon is
284+
// not yet running, so no synchronisation is required for the write.
285+
// Read on the dispatch hot path from fire_handler_exception with no
286+
// lock (single-writer-before-readers contract, same as
287+
// parent->internal_error_handler). If a future task adds a runtime
288+
// setter the writer MUST take hook_table_mutex_ exclusively and the
289+
// reader MUST take it shared -- same pattern as the per-phase vectors.
290+
std::function<::httpserver::hook_action(
291+
const ::httpserver::handler_exception_ctx&)>
292+
handler_exception_alias_;
275293
std::vector<phase_entry<::httpserver::hook_action(
276294
::httpserver::after_handler_ctx&)>>
277295
hooks_after_handler_;

src/httpserver/detail/webserver_impl_dispatch.hpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,35 @@ void fire_route_resolved(
146146
[[nodiscard]] std::optional<::httpserver::http_response>
147147
fire_before_handler(::httpserver::before_handler_ctx& ctx) noexcept;
148148

149+
// TASK-049 -- handler_exception firing helper.
150+
//
151+
// Returns engaged optional iff some hook (user-added or the
152+
// internal_error_handler alias slot) short-circuited with respond_with().
153+
// The caller -- the catch arms in dispatch_resource_handler -- stashes
154+
// that response into mr->response_ and falls through to
155+
// materialize_and_queue_response in finalize_answer.
156+
//
157+
// Chain order:
158+
// 1. User-added hooks in hooks_handler_exception_ (registration order).
159+
// 2. handler_exception_alias_ (the v1 internal_error_handler), if set.
160+
// If no hook short-circuits, returns std::nullopt -- the caller then
161+
// emits the hardcoded empty-body 500 (DR-009 §5.2 point 4) DIRECTLY,
162+
// WITHOUT re-invoking the user internal_error_handler: the alias slot
163+
// has already had its turn at this request, so a second call would
164+
// observably invoke the user code twice.
165+
//
166+
// Per DR-012: a throwing hook in THIS phase is caught, logged via
167+
// log_dispatch_error, and the chain CONTINUES to the next hook -- this
168+
// is the one phase that does not abort to DR-009 §5.2 on a throwing
169+
// hook, because the whole point of the chain IS exception recovery.
170+
// The same containment applies to the alias slot.
171+
//
172+
// noexcept: same contract as fire_before_handler. Snapshot-copy failure
173+
// is logged and degraded to "as if no hooks were registered".
174+
[[nodiscard]] std::optional<::httpserver::http_response>
175+
fire_handler_exception(
176+
const ::httpserver::handler_exception_ctx& ctx) noexcept;
177+
149178
// TASK-031: invoke the user-supplied internal_error_handler safely.
150179
// On success, returns the response it produced. If the user handler
151180
// itself throws, logs generically via log_dispatch_error and returns

src/httpserver/webserver.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,12 @@ namespace httpserver {
140140
* other `std::exception`.
141141
* 6. The `log_error` callback may be invoked concurrently from multiple
142142
* MHD worker threads; user implementations MUST be thread-safe.
143+
* 7. Hook layering (DR-012 §4.10):
144+
* @ref hook_phase::handler_exception hooks fire BEFORE this alias;
145+
* throwing hooks are caught and the chain continues; (4) fires
146+
* without re-invoking the alias on full chain failure.
143147
*
144-
* The contract is the single source of truth for dispatch-time exception
145-
* handling; resource implementations are encouraged to throw rather than
146-
* synthesise an http_response with a 500 status.
148+
* Resources are encouraged to throw rather than synthesise 500s.
147149
**/
148150
class webserver {
149151
public:

0 commit comments

Comments
 (0)