TASK-031: implement DR-009 handler error-propagation contract

Lands the 6-point dispatch error-propagation contract from §5.2 /
PRD-FLG-REQ-002 / DR-009.

API change (v2.0 breaking, intentional):
- `internal_error_handler_t = std::function<http_response(const
  http_request&, std::string_view)>` is a new typedef carrying the
  originating exception's message. `error_handler` continues to serve
  `not_found_handler` and `method_not_allowed_handler` (which receive
  no message). The widening is the smallest deviation consistent with
  action item 2 ("invoke `internal_error_handler` with `e.what()`").

Dispatch path (`webserver_impl::finalize_answer`):
- Replaces the single `catch (...)` with the contract's two-branch
  form: `catch (const std::exception& e)` forwards `e.what()` to the
  user handler; `catch (...)` forwards the literal `"unknown
  exception"`. Each arm logs via `parent->log_error` first.
- New `run_internal_error_handler_safely(mr, msg)` contains a possible
  double-throw from the user handler. On re-throw it logs generically
  and returns a hardcoded empty-body 500 via
  `internal_error_page(mr, "", force_our=true)`.
- `internal_error_page` now takes `(mr, msg, force_our=false)`:
    * `force_our=true`  -> `http_response::empty().with_status(500)`.
    * handler set       -> invoke it with `(*mr->dhr, msg)`.
    * handler unset     -> default body == `msg` so the unset-handler
                            path is informative for debugging.
- The three non-handler-throw call sites that synthesise a 500
  (handler-returned-null, materialize-returned-null, materialize-
  threw) now route through `run_internal_error_handler_safely` so a
  misbehaving user handler can't escape into libmicrohttpd.

`feature_unavailable` (a `std::runtime_error` subclass) lands as a
generic 500 — no special status mapping per DR-009 point 5.

Tests:
- 8 new `dr009_*` integration tests in `test/integ/basic.cpp` pin all
  four acceptance criteria (message-surfaces-in-body,
  message-passed-to-handler, logged-via-error_logger, empty-body-on-
  double-throw, generic-log-on-double-throw, unknown-exception-
  sentinel, feature_unavailable-as-500). Capturing handlers/loggers
  use std::mutex since MHD invokes them concurrently.
- `exception_forces_500`, `untyped_error_forces_500`,
  `file_serving_resource_missing`, `file_serving_resource_dir`
  updated: body assertions changed from `== "Internal Error"` to
  substring-find of the new diagnostic message.
- `internal_error_handler_also_throws`, `builder_internal_error_
  handler`, `create_webserver_explicit_test` updated to the widened
  handler signature. The explicit test gains a static_assert pinning
  that the legacy single-arg signature is now rejected.

Documentation:
- Class-level Doxygen on `webserver` lists the 6 points verbatim with
  cross-references to PRD-FLG-REQ-002 and DR-009.
- Per-setter Doxygen on `create_webserver::internal_error_handler`
  documents the new signature, the unset-handler default, the
  double-throw contract, and `feature_unavailable`'s 500 status.
- Inline comments on `internal_error_page`, `log_dispatch_error`, and
  `run_internal_error_handler_safely` document the contract from the
  implementation side. README pass is M6 (TASK-041).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: etr

specs/tasks/M5-routing-lifecycle/TASK-031.md

@@ -29,4 +29,4 @@ Implement the 6-point error-propagation contract from §5.2 / DR-009 in the disp
 **Related Requirements:** PRD-FLG-REQ-002 (sentinel/throw behavior)
 **Related Decisions:** DR-009, §5.2, AR-007
 
-**Status:** Not Started
+**Status:** In Progress

specs/tasks/_index.md

@@ -113,7 +113,7 @@ Nominally: **13 sequential tasks**, each S–XL. Most other tasks parallelize of
 | TASK-028 | Routing-semantics regression gate | M5 | Done | TASK-027 |
 | TASK-029 | Naming consistency — `stop_and_wait`, `block_ip`/`unblock_ip` | M5 | Done | TASK-014 |
 | TASK-030 | `_handler` suffix renames + `explicit` constructor | M5 | Done | TASK-014 |
-| TASK-031 | Handler error-propagation contract (DR-009) | M5 | Not Started | TASK-027, TASK-030 |
+| TASK-031 | Handler error-propagation contract (DR-009) | M5 | In Progress | TASK-027, TASK-030 |
 | TASK-032 | Thread-safety contract stress test (DR-008) | M5 | Not Started | TASK-027, TASK-031 |
 | TASK-033 | `create_webserver` builder cleanup | M5 | Not Started | TASK-006, TASK-014 |
 | TASK-034 | Build-flag-independent public API + `webserver::features()` | M5 | Not Started | TASK-003, TASK-019, TASK-033 |

src/httpserver/create_webserver.hpp

@@ -30,6 +30,7 @@
 #include <functional>
 #include <limits>
 #include <string>
+#include <string_view>
 #include <utility>
 #include <variant>
 #include <vector>
@@ -43,10 +44,19 @@ namespace httpserver {
 class webserver;
 class http_request;
 
-// TASK-030 / PRD-NAM-REQ-003: the three error-page setters take a function-
-// shaped handler that returns http_response by value, matching the on_*
-// family (detail::lambda_handler) introduced in TASK-025.
+// TASK-030 / PRD-NAM-REQ-003: the not_found_handler and method_not_allowed_handler
+// setters take a function-shaped handler that returns http_response by value,
+// matching the on_* family (detail::lambda_handler) introduced in TASK-025.
 typedef std::function<http_response(const http_request&)> error_handler;
+
+// TASK-031 / DR-009 §5.2: internal_error_handler receives a second argument
+// carrying the originating exception's message — `e.what()` for
+// std::exception, the literal "unknown exception" for non-std throws. The
+// message is a non-owning view; callers MUST NOT capture it past the
+// handler's return (it points into either a caught std::exception's
+// internal storage or a fixed string literal owned by libhttpserver).
+typedef std::function<http_response(const http_request&, std::string_view message)> internal_error_handler_t;
+
 typedef std::function<bool(const std::string&)> validator_ptr;
 typedef std::function<void(const std::string&)> log_access_ptr;
 typedef std::function<void(const std::string&)> log_error_ptr;
@@ -380,7 +390,34 @@ class create_webserver {
          return *this;
      }
 
-     create_webserver& internal_error_handler(error_handler handler) {
+     /**
+      * Set the handler invoked when a registered request handler throws
+      * an uncaught exception.
+      *
+      * Per DR-009 / §5.2 (PRD-FLG-REQ-002), the dispatch path wraps every
+      * handler call in a two-branch try/catch and funnels here:
+      *
+      *  - `std::exception`        -> @p handler is invoked with
+      *                               `e.what()` as the message argument.
+      *  - non-std::exception      -> @p handler is invoked with the
+      *                               literal string `"unknown exception"`.
+      *  - @p handler unset        -> a default 500 with the message in
+      *                               the response body is sent and the
+      *                               event is logged via log_error.
+      *  - @p handler itself throws -> a hardcoded 500 with an EMPTY body
+      *                               is sent and the event is logged via
+      *                               log_error.
+      *
+      * `feature_unavailable` is a `std::runtime_error` subclass: it lands
+      * here with the default 500 status, not a special 503.
+      *
+      * Note: @p handler may be invoked concurrently from multiple MHD
+      * worker threads. It MUST be thread-safe.
+      *
+      * @param handler invoked with (request, message); returns the
+      *                http_response to send (status / headers / body).
+     **/
+     create_webserver& internal_error_handler(internal_error_handler_t handler) {
          _internal_error_handler = std::move(handler);
          return *this;
      }
@@ -528,7 +565,7 @@ class create_webserver {
      bool _tcp_nodelay = false;
      error_handler _not_found_handler = nullptr;
      error_handler _method_not_allowed_handler = nullptr;
-     error_handler _internal_error_handler = nullptr;
+     internal_error_handler_t _internal_error_handler = nullptr;
      file_cleanup_callback_ptr _file_cleanup_callback = nullptr;
      auth_handler_ptr _auth_handler = nullptr;
      std::vector<std::string> _auth_skip_paths;

src/httpserver/detail/webserver_impl.hpp

@@ -53,6 +53,7 @@
 #include <set>
 #include <shared_mutex>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -305,8 +306,43 @@ class webserver_impl {
     // owning webserver (via `parent`).
     std::shared_ptr<::httpserver::http_response> not_found_page(modded_request* mr) const;
     std::shared_ptr<::httpserver::http_response> method_not_allowed_page(modded_request* mr) const;
-    std::shared_ptr<::httpserver::http_response> internal_error_page(modded_request* mr,
-                                                       bool force_our = false) const;
+    // TASK-031: error-propagation entry point. @p msg carries the
+    // originating exception's text (e.what() for std::exception, the
+    // sentinel "unknown exception" for non-std throws, or a fixed
+    // internal-diagnostic string for the few non-handler-throw call
+    // sites that synthesise a 500 with no exception in flight).
+    //
+    // Behaviour matches DR-009:
+    //   - parent->internal_error_handler set, !force_our:
+    //       invoke it with (*mr->dhr, msg) and return the result.
+    //   - force_our=true: return a hardcoded 500 with an EMPTY body
+    //       (the "double-fault" fallback used when the user handler
+    //       itself threw).
+    //   - otherwise (no handler set, !force_our): return a default
+    //       500 whose body surfaces @p msg, so the unset-handler
+    //       default is informative.
+    //
+    // Throws nothing on its own; if parent->internal_error_handler
+    // throws, that exception propagates to the caller. Callers in the
+    // handler-throw path use run_internal_error_handler_safely() to
+    // contain that double-fault.
+    std::shared_ptr<::httpserver::http_response> internal_error_page(
+        modded_request* mr,
+        std::string_view msg,
+        bool force_our = false) const;
+
+    // TASK-031: log @p msg via parent->log_error if a logger is configured.
+    // Swallows any exception thrown by the logger -- dispatch must never
+    // re-enter the catch from inside its own catch.
+    void log_dispatch_error(std::string_view msg) const;
+
+    // TASK-031: invoke the user-supplied internal_error_handler safely.
+    // On success, returns the response it produced. If the user handler
+    // itself throws, logs generically via log_dispatch_error and returns
+    // a hardcoded empty-body 500.
+    std::shared_ptr<::httpserver::http_response>
+        run_internal_error_handler_safely(modded_request* mr,
+                                          std::string_view msg) const;
     bool should_skip_auth(const std::string& path) const;
     void invalidate_route_cache();
 

src/httpserver/webserver.hpp

@@ -76,6 +76,32 @@ namespace httpserver {
 
 /**
  * Class representing the webserver. Main class of the apis.
+ *
+ * ### Handler error-propagation contract (DR-009 / §5.2 / PRD-FLG-REQ-002)
+ *
+ * Every registered request handler is invoked from the dispatch path under
+ * a two-branch try/catch. The contract:
+ *
+ *   1. The handler call is wrapped in
+ *      `try { ... } catch (const std::exception& e) { ... } catch (...) { ... }`.
+ *   2. On `std::exception`: the message is logged via the configured
+ *      `log_error` callback, then `internal_error_handler` is invoked with
+ *      `e.what()`. The response it returns is sent on the wire (default
+ *      500 with the message in the body when no handler is configured).
+ *   3. On non-`std::exception` (e.g. `throw 42`): same path with the
+ *      message replaced by the literal string `"unknown exception"`.
+ *   4. If `internal_error_handler` itself throws while servicing 2 or 3,
+ *      the failure is logged generically and a hardcoded 500 with an
+ *      EMPTY body is sent. No exception ever escapes into libmicrohttpd.
+ *   5. `feature_unavailable` (a `std::runtime_error` subclass) is NOT
+ *      mapped to a special status: it lands as a generic 500 like any
+ *      other `std::exception`.
+ *   6. The `log_error` callback may be invoked concurrently from multiple
+ *      MHD worker threads; user implementations MUST be thread-safe.
+ *
+ * The contract is the single source of truth for dispatch-time exception
+ * handling; resource implementations are encouraged to throw rather than
+ * synthesise an http_response with a 500 status.
 **/
 class webserver {
  public:
@@ -468,7 +494,7 @@ class webserver {
      const bool tcp_nodelay;
      const error_handler not_found_handler;
      const error_handler method_not_allowed_handler;
-     const error_handler internal_error_handler;
+     const internal_error_handler_t internal_error_handler;
      const file_cleanup_callback_ptr file_cleanup_callback;
      const auth_handler_ptr auth_handler;
      const std::vector<std::string> auth_skip_paths;

src/webserver.cpp

@@ -1610,14 +1610,60 @@ std::shared_ptr<http_response> webserver_impl::method_not_allowed_page(detail::m
     }
 }
 
-std::shared_ptr<http_response> webserver_impl::internal_error_page(detail::modded_request* mr, bool force_our) const {
-    if (parent->internal_error_handler != nullptr && !force_our) {
-        return std::make_shared<http_response>(parent->internal_error_handler(*mr->dhr));
-    } else {
+std::shared_ptr<http_response> webserver_impl::internal_error_page(
+    detail::modded_request* mr,
+    std::string_view msg,
+    bool force_our) const {
+    // TASK-031 / DR-009 §5.2 point 4: the double-fault fallback. Used when
+    // the user-supplied internal_error_handler itself threw or when the
+    // belt-and-suspenders site after get_raw_response_with_fallback fires.
+    // The body is intentionally empty and the message is intentionally
+    // ignored.
+    if (force_our) {
         return std::make_shared<http_response>(
-            http_response::string(std::string{constants::GENERIC_ERROR})
+            http_response::empty()
                 .with_status(http_utils::http_internal_server_error));
     }
+    // §5.2 point 2/3: invoke the user handler with the originating message.
+    if (parent->internal_error_handler != nullptr) {
+        return std::make_shared<http_response>(
+            parent->internal_error_handler(*mr->dhr, msg));
+    }
+    // No handler configured: surface the message in the default body so
+    // the unset-handler path is informative for debugging. Operators who
+    // need a fixed body can wire a constant-returning handler.
+    return std::make_shared<http_response>(
+        http_response::string(std::string{msg})
+            .with_status(http_utils::http_internal_server_error));
+}
+
+void webserver_impl::log_dispatch_error(std::string_view msg) const {
+    if (parent->log_error == nullptr) {
+        return;
+    }
+    // A misbehaving user logger must not poison the catch from inside the
+    // catch. Swallow any exception it throws; we have no recovery beyond
+    // dropping the log line.
+    try {
+        parent->log_error(std::string(msg));
+    } catch (...) {
+        // Intentionally suppressed.
+    }
+}
+
+std::shared_ptr<http_response>
+webserver_impl::run_internal_error_handler_safely(
+    detail::modded_request* mr,
+    std::string_view msg) const {
+    try {
+        return internal_error_page(mr, msg, /*force_our=*/false);
+    } catch (...) {
+        // §5.2 point 4: the user handler itself threw. Log generically
+        // and return an empty-body 500. No exception escapes from here.
+        log_dispatch_error("internal_error_handler threw; "
+                           "sending hardcoded empty-body 500");
+        return internal_error_page(mr, "", /*force_our=*/true);
+    }
 }
 
 void webserver_impl::invalidate_route_cache() {
@@ -1871,7 +1917,11 @@ struct MHD_Response* webserver_impl::get_raw_response_with_fallback(detail::modd
     try {
         struct MHD_Response* raw = materialize_response(mr->dhrs.get());
         if (raw == nullptr) {
-            mr->dhrs = internal_error_page(mr);
+            // TASK-031: no exception was thrown, but the body materializer
+            // returned null. Route through the safe internal-error path so
+            // a misbehaving user handler can't escape.
+            mr->dhrs = run_internal_error_handler_safely(
+                mr, "materialize_response returned null");
             raw = materialize_response(mr->dhrs.get());
         }
         return raw;
@@ -1882,9 +1932,19 @@ struct MHD_Response* webserver_impl::get_raw_response_with_fallback(detail::modd
         } catch(...) {
             return nullptr;
         }
+    } catch(const std::exception& e) {
+        log_dispatch_error(std::string("materialize threw: ") + e.what());
+        try {
+            mr->dhrs = run_internal_error_handler_safely(mr, e.what());
+            return materialize_response(mr->dhrs.get());
+        } catch(...) {
+            return nullptr;
+        }
     } catch(...) {
+        log_dispatch_error("materialize threw unknown exception");
         try {
-            mr->dhrs = internal_error_page(mr);
+            mr->dhrs = run_internal_error_handler_safely(mr,
+                                                         "unknown exception");
             return materialize_response(mr->dhrs.get());
         } catch(...) {
             return nullptr;
@@ -2072,7 +2132,12 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection, struct de
                 // shared_ptr has no operator->*, so call as ((*ptr).*pmf)(...).
                 mr->dhrs = ((*hrm).*(mr->callback))(*mr->dhr);  // copy in memory (move in case)
                 if (mr->dhrs.get() == nullptr || mr->dhrs->get_status() == -1) {
-                    mr->dhrs = internal_error_page(mr);
+                    // TASK-031: no exception was thrown, but the handler
+                    // returned a null/invalid response. Route through the
+                    // safe internal-error path so a misbehaving user
+                    // handler can't escape into libmicrohttpd.
+                    mr->dhrs = run_internal_error_handler_safely(
+                        mr, "handler returned null response");
                 }
             } else {
                 mr->dhrs = method_not_allowed_page(mr);
@@ -2096,23 +2161,31 @@ MHD_Result webserver_impl::finalize_answer(MHD_Connection* connection, struct de
                     mr->dhrs->with_header(http_utils::http_header_allow, header_value);
                 }
             }
-        } catch(...) {
-            // The user-supplied internal_error_handler may itself throw;
-            // fall back to the built-in error page in that case (force_our=true)
-            // so we never let exceptions escape into libmicrohttpd.
-            try {
-                mr->dhrs = internal_error_page(mr);
-            } catch(...) {
-                mr->dhrs = internal_error_page(mr, true);
-            }
+        } catch (const std::exception& e) {
+            // TASK-031 / DR-009 §5.2 point 2: handler threw std::exception.
+            // Log via error_logger, forward e.what() to internal_error_handler.
+            // run_internal_error_handler_safely contains a possible
+            // re-throw from the user handler (point 4).
+            log_dispatch_error(std::string("dispatch: handler threw "
+                                           "std::exception: ") + e.what());
+            mr->dhrs = run_internal_error_handler_safely(mr, e.what());
+        } catch (...) {
+            // §5.2 point 3: handler threw non-std::exception. Same flow
+            // as the std::exception arm but with the sentinel message.
+            log_dispatch_error("dispatch: handler threw unknown exception");
+            mr->dhrs = run_internal_error_handler_safely(mr,
+                                                         "unknown exception");
         }
     } else if (mr->dhrs == nullptr) {
         mr->dhrs = not_found_page(mr);
     }
 
     raw_response = get_raw_response_with_fallback(mr);
     if (raw_response == nullptr) {
-        mr->dhrs = internal_error_page(mr, true);
+        // Belt-and-suspenders: even get_raw_response_with_fallback's
+        // own try/catch couldn't produce a response. Force the empty-body
+        // 500 fallback so MHD always has something to queue.
+        mr->dhrs = internal_error_page(mr, "", /*force_our=*/true);
         raw_response = materialize_response(mr->dhrs.get());
     }
     decorate_mhd_response(raw_response, *mr->dhrs);