From 66dc3708e63fc8bf6681ce7d9d966e196bfb597b Mon Sep 17 00:00:00 2001 From: will wade Date: Sun, 28 Jun 2026 23:30:01 +0100 Subject: [PATCH 1/2] fix: harden C API boundary exception handling (issue #34) Collapses the prior fixup chain (helpers + clang-format + placement) into one commit and drops the unrelated CODEOWNERS commit (landed separately in #36). Rebased onto current main. Standardises every C-API entry point that can throw on the log callback: - Adds a single noexcept, allocation-free helper log_boundary_error() that formats ": " into a fixed stack buffer via snprintf. Catch handlers must not throw: a std::string concat hitting bad_alloc would re-violate the boundary and, for void setters, cannot recover (RFC 0009 Amendment 2 requires this property). - Guards the four setters (dasher_set_bool/long/string_parameter, dasher_set_speed_percent) which call broadcasting observer code. - Upgrades the three getters to catch std::exception + ... and route through the callback, replacing fprintf(stderr,...) (lost on iOS keyboard extensions / WASM / embedded) and the bad_variant_access-only catch that let other exception types escape. - Logs dasher_set_screen_size Realize() failures instead of silently swallowing them. - Updates CONTRIBUTING Rule 4 with the void-function policy and a pointer to the helper. Verified: clang-format clean on Src/CAPI.cpp; clang + clang-tidy build of libdasher.so passes with no tidy findings. Signed-off-by: will wade --- CONTRIBUTING.md | 31 +++++++++++++++- src/CAPI.cpp | 95 ++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 112 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 537e434a..9cafb2eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,9 +96,15 @@ all frontends (Swift, C#, WASM, Rust). It must be flawless: catch all exceptions and return an error code. - **No C++ types cross the boundary.** Use `const char*`, `int`, `int64_t`, and raw structs — never `std::string` or `std::vector`. +- **Use the log callback for diagnostics.** When exceptions occur in void + functions or during initialization, log them via `ctx->logCb` if registered, + then return or continue safely. Never use `fprintf(stderr, ...)` — it + doesn't work on embedded targets. Prefer the internal `log_boundary_error()` + helper (noexcept, allocation-free) so a catch handler can never itself throw + across the boundary. ```cpp -// GOOD +// GOOD — function with return value DASHER_API int dasher_get_offset(dasher_ctx* ctx) { if (!ctx || !ctx->realized) return -1; try { @@ -108,10 +114,33 @@ DASHER_API int dasher_get_offset(dasher_ctx* ctx) { } } +// GOOD — void function: log via callback, then return +DASHER_API void dasher_set_bool_parameter(dasher_ctx* ctx, int key, int value) { + if (!ctx || !ctx->intf) return; + try { + ctx->intf->SetBoolParameter(static_cast(key), value != 0); + } catch (const std::exception& e) { + if (ctx->logCb && 3 >= ctx->logCbMinLevel) + ctx->logCb(3, e.what(), ctx->logCbUserData); + } catch (...) { + if (ctx->logCb && 3 >= ctx->logCbMinLevel) + ctx->logCb(3, "unknown exception", ctx->logCbUserData); + } +} + // BAD — exception crosses extern "C" DASHER_API int dasher_get_offset(dasher_ctx* ctx) { return ctx->intf->GetModel()->GetOffset(); // Can throw! } + +// BAD — fprintf doesn't work on iOS/WASM/embedded +DASHER_API void dasher_set_bool_parameter(dasher_ctx* ctx, int key, int value) { + try { + ctx->intf->SetBoolParameter(static_cast(key), value != 0); + } catch (const std::exception& e) { + fprintf(stderr, "Exception: %s\n", e.what()); // Lost on embedded! + } +} ``` ### Rule 5: Zero Compiler Warnings diff --git a/src/CAPI.cpp b/src/CAPI.cpp index d7651332..c06ba092 100644 --- a/src/CAPI.cpp +++ b/src/CAPI.cpp @@ -538,6 +538,26 @@ struct dasher_ctx { }; }; +// ── C API Boundary Exception Helpers ──────────────────────────────────────── +// Enforces Rule 4: never throw across the C API boundary. All exceptions are +// caught and reported via the log callback at level 3 (ERROR) when registered +// and at/above min_level; otherwise silently discarded. +// +// noexcept and allocation-free (fixed buffer + snprintf). A catch handler that +// itself throws — e.g. a std::string concat hitting bad_alloc — re-violates the +// boundary and, for void setters, cannot recover. RFC 0009 Amendment 2 requires +// this property so engine fault context reliably reaches the frontend ring +// buffer before the function returns. + +static void log_boundary_error(dasher_ctx* ctx, const char* context, const char* detail) noexcept { + if (!ctx || !ctx->logCb || 3 /*ERROR*/ < ctx->logCbMinLevel) return; + char buf[256]; + const int n = snprintf(buf, sizeof(buf), "%s: %s", context ? context : "", detail ? detail : ""); + if (n < 0) return; // encoding error — nothing useful to report + // snprintf always null-terminates (size > 0), so buf is valid even if truncated. + ctx->logCb(3, buf, ctx->logCbUserData); +} + // ── C API implementation ────────────────────────────────────────────────── // Appearance model helpers (RFC 0007). Defined outside `extern "C"` because they @@ -745,7 +765,10 @@ DASHER_API void dasher_set_screen_size(dasher_ctx* ctx, int width, int height) { if (!ctx->realized) { try { ctx->intf->Realize(nowMs()); - } catch (...) { // NOLINT(bugprone-empty-catch) + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_set_screen_size: Realize failed", e.what()); + } catch (...) { + log_boundary_error(ctx, "dasher_set_screen_size: Realize failed", "unknown exception"); } ctx->realized = true; @@ -922,48 +945,88 @@ DASHER_API int dasher_get_speed_percent(dasher_ctx* ctx) { DASHER_API void dasher_set_speed_percent(dasher_ctx* ctx, int percent) { if (!ctx || !ctx->intf) return; - const double base = 160.0; - const int clamped = (percent < 20) ? 20 : (percent > 400) ? 400 : percent; - long bitrate = static_cast(lround_int(clamped / 100.0 * base)); - if (bitrate < 1) bitrate = 1; - ctx->intf->SetLongParameter(Dasher::LP_MAX_BITRATE, bitrate); + try { + const double base = 160.0; + const int clamped = (percent < 20) ? 20 : (percent > 400) ? 400 : percent; + long bitrate = static_cast(lround_int(clamped / 100.0 * base)); + if (bitrate < 1) bitrate = 1; + ctx->intf->SetLongParameter(Dasher::LP_MAX_BITRATE, bitrate); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_set_speed_percent", e.what()); + } catch (...) { + log_boundary_error(ctx, "dasher_set_speed_percent", "unknown exception"); + } } DASHER_API int dasher_get_bool_parameter(dasher_ctx* ctx, int key) { if (!ctx || !ctx->intf) return 0; try { return ctx->intf->GetBoolParameter(static_cast(key)) ? 1 : 0; - } catch (const std::bad_variant_access&) { - fprintf(stderr, "DASHER: bad_variant_access in get_bool_parameter key=%d\n", key); + } catch (const std::exception& e) { + char context[96]; + snprintf(context, sizeof(context), "dasher_get_bool_parameter key=%d", key); + log_boundary_error(ctx, context, e.what()); + return 0; + } catch (...) { + char context[96]; + snprintf(context, sizeof(context), "dasher_get_bool_parameter key=%d", key); + log_boundary_error(ctx, context, "unknown exception"); return 0; } } DASHER_API void dasher_set_bool_parameter(dasher_ctx* ctx, int key, int value) { if (!ctx || !ctx->intf) return; - ctx->intf->SetBoolParameter(static_cast(key), value != 0); + try { + ctx->intf->SetBoolParameter(static_cast(key), value != 0); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_set_bool_parameter", e.what()); + } catch (...) { + log_boundary_error(ctx, "dasher_set_bool_parameter", "unknown exception"); + } } DASHER_API long dasher_get_long_parameter(dasher_ctx* ctx, int key) { if (!ctx || !ctx->intf) return 0; try { return ctx->intf->GetLongParameter(static_cast(key)); - } catch (const std::bad_variant_access&) { - fprintf(stderr, "DASHER: bad_variant_access in get_long_parameter key=%d\n", key); + } catch (const std::exception& e) { + char context[96]; + snprintf(context, sizeof(context), "dasher_get_long_parameter key=%d", key); + log_boundary_error(ctx, context, e.what()); + return 0; + } catch (...) { + char context[96]; + snprintf(context, sizeof(context), "dasher_get_long_parameter key=%d", key); + log_boundary_error(ctx, context, "unknown exception"); return 0; } } DASHER_API void dasher_set_long_parameter(dasher_ctx* ctx, int key, long value) { if (!ctx || !ctx->intf) return; - ctx->intf->SetLongParameter(static_cast(key), value); + try { + ctx->intf->SetLongParameter(static_cast(key), value); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_set_long_parameter", e.what()); + } catch (...) { + log_boundary_error(ctx, "dasher_set_long_parameter", "unknown exception"); + } } DASHER_API const char* dasher_get_string_parameter(dasher_ctx* ctx, int key) { if (!ctx || !ctx->intf) return ""; try { ctx->tlString = ctx->intf->GetStringParameter(static_cast(key)); + } catch (const std::exception& e) { + char context[96]; + snprintf(context, sizeof(context), "dasher_get_string_parameter key=%d", key); + log_boundary_error(ctx, context, e.what()); + ctx->tlString = ""; } catch (...) { + char context[96]; + snprintf(context, sizeof(context), "dasher_get_string_parameter key=%d", key); + log_boundary_error(ctx, context, "unknown exception"); ctx->tlString = ""; } return ctx->tlString.c_str(); @@ -971,7 +1034,13 @@ DASHER_API const char* dasher_get_string_parameter(dasher_ctx* ctx, int key) { DASHER_API void dasher_set_string_parameter(dasher_ctx* ctx, int key, const char* value) { if (!ctx || !ctx->intf || !value) return; - ctx->intf->SetStringParameter(static_cast(key), value); + try { + ctx->intf->SetStringParameter(static_cast(key), value); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_set_string_parameter", e.what()); + } catch (...) { + log_boundary_error(ctx, "dasher_set_string_parameter", "unknown exception"); + } } // Color utility functions From fd3cb693da0d5de2a8077c2dd55e1574a6ed3cd6 Mon Sep 17 00:00:00 2001 From: will wade Date: Sun, 28 Jun 2026 23:36:23 +0100 Subject: [PATCH 2/2] feat(capi): wrap per-frame hot path + add engine error flag (RFC 0009 A2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parameter accessors were standardised in #35; the remaining and most valuable gap was the per-frame hot path — the least guarded entry points and the most likely to hit engine bugs. C++ exceptions escaping dasher_frame / dasher_mouse_* / dasher_key_event across extern "C" left the frontend engine-log ring buffer with no fault context, so a crash report showed where the frontend was, not what the engine was doing (RFC 0009 Amendment 2). - Wrap dasher_frame, dasher_mouse_move/down/up, dasher_key_event in try/catch; route caught exceptions through log_boundary_error() at level 3 (reused from #35) so the fault reaches the frontend ring buffer before the function returns. - Add bool dasher_ctx::engineError, latched on catch. Once set, those five entry points no-op — the engine is indeterminate after a mid-frame throw and continuing would risk a hard SEGV this cannot catch. - Expose via int dasher_has_engine_error(dasher_ctx*) (the context is opaque). Frontend contract: on true, stop calling frame, surface an error, then dasher_destroy() + dasher_create(). Not cleared by dasher_reset(). What this does NOT catch: SEGV/SIGBUS, stack overflow, destructor throws during unwinding, and DASHER_ASSERT (a no-op under NDEBUG, not an abort). Those remain the per-platform signal handlers job per the base RFC; the log tail still helps because the last successful frame logs precede the signal. Depends on #35 (log_boundary_error helper). Verified: clang-format clean on src/CAPI.cpp and src/dasher.h; clang + clang-tidy build of libdasher.so passes with no tidy findings. Signed-off-by: will wade --- src/CAPI.cpp | 96 ++++++++++++++++++++++++++++++++++++++++------------ src/dasher.h | 8 +++++ 2 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/CAPI.cpp b/src/CAPI.cpp index c06ba092..6ac305fb 100644 --- a/src/CAPI.cpp +++ b/src/CAPI.cpp @@ -343,6 +343,13 @@ struct dasher_ctx { std::string tlString; bool realized = false; bool mouseDown = false; + // Latched true when a C++ exception was caught at the C-API boundary of a + // per-frame entry point (frame/mouse/key). Once set, those entry points + // no-op until the engine is destroyed and recreated — the engine state is + // indeterminate after a mid-frame throw. Frontends query this via + // dasher_has_engine_error(). Per RFC 0009 Amendment 2; not cleared by + // dasher_reset (only by recreating the context). + bool engineError = false; std::string pendingAlphabet; std::string dataDir; std::string userDir; @@ -787,36 +794,70 @@ DASHER_API void dasher_set_screen_size(dasher_ctx* ctx, int width, int height) { DASHER_API void dasher_mouse_move(dasher_ctx* ctx, float x, float y) { if (!ctx || !ctx->input) return; - ctx->input->SetPosition(x, y); + if (ctx->engineError) return; + try { + ctx->input->SetPosition(x, y); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_mouse_move", e.what()); + ctx->engineError = true; + } catch (...) { + log_boundary_error(ctx, "dasher_mouse_move", "unknown exception"); + ctx->engineError = true; + } } DASHER_API void dasher_mouse_down(dasher_ctx* ctx) { if (!ctx || !ctx->intf) return; + if (ctx->engineError) return; if (ctx->mouseDown) return; ctx->mouseDown = true; - - // In circle start mode, clicking should NOT start/stop Dasher — - // only hovering inside the circle should. (Steve Saling feedback) - if (ctx->intf->GetLongParameter(Dasher::LP_START_MODE) == Dasher::Options::StartMode::circle_start) return; - - ctx->intf->SetBoolParameter(Dasher::BP_START_MOUSE, true); - ctx->intf->KeyDown(nowMs(), Dasher::Keys::Primary_Input); + try { + // In circle start mode, clicking should NOT start/stop Dasher — + // only hovering inside the circle should. (Steve Saling feedback) + if (ctx->intf->GetLongParameter(Dasher::LP_START_MODE) == Dasher::Options::StartMode::circle_start) return; + ctx->intf->SetBoolParameter(Dasher::BP_START_MOUSE, true); + ctx->intf->KeyDown(nowMs(), Dasher::Keys::Primary_Input); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_mouse_down", e.what()); + ctx->engineError = true; + } catch (...) { + log_boundary_error(ctx, "dasher_mouse_down", "unknown exception"); + ctx->engineError = true; + } } DASHER_API void dasher_mouse_up(dasher_ctx* ctx) { if (!ctx || !ctx->intf) return; + if (ctx->engineError) return; if (!ctx->mouseDown) return; ctx->mouseDown = false; - ctx->intf->KeyUp(nowMs(), Dasher::Keys::Primary_Input); + try { + ctx->intf->KeyUp(nowMs(), Dasher::Keys::Primary_Input); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_mouse_up", e.what()); + ctx->engineError = true; + } catch (...) { + log_boundary_error(ctx, "dasher_mouse_up", "unknown exception"); + ctx->engineError = true; + } } DASHER_API void dasher_key_event(dasher_ctx* ctx, int key, int pressed) { if (!ctx || !ctx->intf) return; - auto vk = static_cast(key); - if (pressed) { - ctx->intf->KeyDown(nowMs(), vk); - } else { - ctx->intf->KeyUp(nowMs(), vk); + if (ctx->engineError) return; + try { + auto vk = static_cast(key); + if (pressed) { + ctx->intf->KeyDown(nowMs(), vk); + } else { + ctx->intf->KeyUp(nowMs(), vk); + } + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_key_event", e.what()); + ctx->engineError = true; + } catch (...) { + log_boundary_error(ctx, "dasher_key_event", "unknown exception"); + ctx->engineError = true; } } @@ -828,15 +869,28 @@ DASHER_API void dasher_frame(dasher_ctx* ctx, int64_t time_ms, int** out_command if (out_string_count) *out_string_count = 0; if (!ctx || !ctx->intf || !ctx->screen || !ctx->realized) return; + if (ctx->engineError) return; - ctx->screen->BeginFrame(); - ctx->intf->NewFrame(static_cast((time_ms > 0) ? time_ms : 0), true); - ctx->screen->BuildStringPtrs(); + try { + ctx->screen->BeginFrame(); + ctx->intf->NewFrame(static_cast((time_ms > 0) ? time_ms : 0), true); + ctx->screen->BuildStringPtrs(); + + if (out_commands) *out_commands = const_cast(reinterpret_cast(ctx->screen->GetCommands())); + if (out_command_count) *out_command_count = ctx->screen->GetCommandCount(); + if (out_strings) *out_strings = const_cast(ctx->screen->GetStringPtrs()); + if (out_string_count) *out_string_count = ctx->screen->GetStringCount(); + } catch (const std::exception& e) { + log_boundary_error(ctx, "dasher_frame", e.what()); + ctx->engineError = true; + } catch (...) { + log_boundary_error(ctx, "dasher_frame", "unknown exception"); + ctx->engineError = true; + } +} - if (out_commands) *out_commands = const_cast(reinterpret_cast(ctx->screen->GetCommands())); - if (out_command_count) *out_command_count = ctx->screen->GetCommandCount(); - if (out_strings) *out_strings = const_cast(ctx->screen->GetStringPtrs()); - if (out_string_count) *out_string_count = ctx->screen->GetStringCount(); +DASHER_API int dasher_has_engine_error(dasher_ctx* ctx) { + return (ctx && ctx->engineError) ? 1 : 0; } DASHER_API const char* dasher_get_output_text(dasher_ctx* ctx) { diff --git a/src/dasher.h b/src/dasher.h index c8929a0b..886b767f 100644 --- a/src/dasher.h +++ b/src/dasher.h @@ -93,6 +93,14 @@ DASHER_API void dasher_key_event(dasher_ctx* ctx, int key, int pressed); DASHER_API void dasher_frame(dasher_ctx* ctx, int64_t time_ms, int** out_commands, int* out_command_count, char*** out_strings, int* out_string_count); +// Engine fault flag. Returns 1 if a C++ exception was caught at the boundary +// of dasher_frame / dasher_mouse_* / dasher_key_event, leaving the engine in +// an indeterminate state; 0 otherwise. When true, those per-frame entry points +// no-op and the frontend must stop calling them, surface an error, then +// dasher_destroy() + dasher_create() a fresh context. Not cleared by +// dasher_reset(). See RFC 0009 Amendment 2. +DASHER_API int dasher_has_engine_error(dasher_ctx* ctx); + // Get/set output text (characters entered so far). // Returned pointer is valid until the next API call on this context. DASHER_API const char* dasher_get_output_text(dasher_ctx* ctx);