From b5cd0296d9b33f54905ad5d4e94ea6f195ced3b9 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 23 Sep 2026 23:07:30 +0200 Subject: [PATCH 1/4] =?UTF-8?q?core+journal:=20a=20journal=20sink=20that?= =?UTF-8?q?=20refuses=20a=20success=20made=20the=20framework=20report=20?= =?UTF-8?q?=E2=80=94=20and=20record=20=E2=80=94=20a=20committed=20write=20?= =?UTF-8?q?as=20rejected=20(fixes=20#796)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both sites that actually run `Model::execute` called `recordActionSuccess` from inside the `try` whose `catch (const std::exception&)` exists to report *execution* failures: `ActionDispatcher::registerAction`'s runner and `Bridge::executeVia`'s `localOp`. `ActionTraits::resultToJson` sat inside that same `try`. `IActionLog::append` is required to throw when the entry did not reach its backend — the return type is `void`, so it is the only channel the interface gives an implementation, and `file_action_log.hpp` throws from 18 sites. A journal file on a full disk therefore produced three wrong answers at once, after the model's mutation had already committed: dispatch threw: journal sink unavailable model.committed = 1, model.balance = 10 journal entries = 1 outcome=Failed error=journal sink unavailable result= The caller was told a durable write was rejected and could retry it, the audit trail gained an `Outcome::Failed` entry for a mutation that committed, and that entry's `error` field blamed the action for an infrastructure fault. `Model::execute` is now the only call inside that `try` — it is the only one whose failure means the action was rejected. Serialising the result and appending the entry run after it, and a throw from either surfaces as the new `morph::model::ActionRecordingError`: `what()` is "action executed but was not recorded: ", `cause()` is the underlying message, `result()` is the committed action's result JSON. The caller still learns the recording failed; what changed is what it is told, which is now true. Deriving from `std::runtime_error` leaves every existing `catch (const std::exception&)` path working — `RemoteServer` still replies `err`, `LocalBackend` still rejects the `Completion` through `onError`. `resultToJson` moved out too: a result type whose serialisation throws produced the identical three symptoms with no journal involved. When it is what threw, no entry is written at all — a `Succeeded` entry carries the result by definition and there is none to carry, so the caller is told and the audit trail is left with a gap rather than an assertion that the action failed. `OutboxRelay::relay()` is untouched and still depends on `append`/`flush` throwing: it calls them directly, and marks a row relayed only after the sink returned normally. Tests (`tests/test_action_log.cpp`): a `SuccessRefusingLog` that throws on the `Succeeded` append and accepts the `Failed` one, plus a hand-written `ActionTraits` whose `resultToJson` throws. Five cases across both dispatch paths. Each assertion was reddened by mutating the fix back: - with `resultToJson`+`recordActionSuccess` returned to the execution `try`, the two refusing-sink cases fail on `seen.recordingError`, `seen.what == "action executed but was not recorded: journal sink unavailable"`, `seen.cause`, `seen.result == "10"`, `log->entries().empty()` (false — the `Failed` entry is there), `log->offered().size() == 1` (2) and `entry.outcome != Outcome::Failed` (1 != 1); the unserialisable-result case fails on `seen.recordingError`, `seen.cause` and `log->entries().empty()`; - with the execution `catch` changed to throw `ActionRecordingError` instead of rethrowing, the two regression-guard cases fail on `CHECK_FALSE(seen.recordingError)` and `seen.what == "insufficient funds"` (got "action executed but was not recorded: insufficient funds"). Full `ctest` on `clang-debug` (clang 22.1.8, Linux): 1782/1782 passed. Specs updated in the same commit: `docs/spec/journal/journal.md` gains "A refused recording is not an execution failure"; `docs/spec/core/registry.md` and `docs/spec/core/bridge.md` no longer describe the success record as living inside the execution `try`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- docs/spec/core/bridge.md | 24 ++- docs/spec/core/registry.md | 24 ++- docs/spec/journal/journal.md | 51 ++++++ include/morph/core/bridge.hpp | 58 +++--- include/morph/core/registry.hpp | 129 +++++++++++--- tests/test_action_log.cpp | 306 ++++++++++++++++++++++++++++++++ 6 files changed, 535 insertions(+), 57 deletions(-) diff --git a/docs/spec/core/bridge.md b/docs/spec/core/bridge.md index 9a0844278..b07f7c444 100644 --- a/docs/spec/core/bridge.md +++ b/docs/spec/core/bridge.md @@ -194,14 +194,20 @@ throw at runtime means `LocalBackend` was used in a build that promised never to — a configuration error, not a normal failure mode. Otherwise (the default, non-`MORPH_CLIENT_ONLY` build), `Model::execute(*action)` -itself is wrapped in a `try`/`catch (const std::exception&)`: on success it -records a journal `LogEntry` with `outcome = Outcome::Succeeded` for loggable -actions; on a throw it records `outcome = Outcome::Failed` (`error = -exc.what()`, `result` empty) for the same actions and rethrows unchanged, so -the exception still resolves the `Completion` through `onError` exactly as -before — the journal entry is a side effect of the attempt, not a change to -error propagation. Mirrors `ActionDispatcher::registerAction`'s runner -(`registry.md`) for remote topologies. See [journal.md, +itself — and nothing else — is wrapped in a `try`/`catch (const std::exception&)`: +on a throw it records `outcome = Outcome::Failed` (`error = exc.what()`, +`result` empty) for loggable actions and rethrows unchanged, so the exception +still resolves the `Completion` through `onError` exactly as before — the +journal entry is a side effect of the attempt, not a change to error +propagation. Serialising the result and recording `outcome = Outcome::Succeeded` +run after that `try`, because by then the mutation has committed: a sink whose +`append` throws, or a result that will not serialise, resolves the `Completion` +through `onError` with `morph::model::ActionRecordingError` rather than being +reported — and journaled — as the model refusing the action. Mirrors +`ActionDispatcher::registerAction`'s runner (`registry.md`) for remote +topologies. See [journal.md, "A refused recording is not an execution +failure"](../journal/journal.md#a-refused-recording-is-not-an-execution-failure) +and [journal.md, "Outcome"](../journal/journal.md#logentry--one-recorded-action-execution) for the full field/replay semantics. @@ -1105,7 +1111,7 @@ make teardown order-independent.) | `registerHandler(binding)` | `void registerHandler(const shared_ptr&)` | Pre-built binding. Same async-preferring behavior. | | `switchBackend` | `void switchBackend(unique_ptr)` / `void switchBackend(shared_ptr)` | Pushes the current default session onto the new backend via `setSession` before staging. Stages all re-registrations through `bindModel` on the new backend, commits (publishes new ids + swaps) only if all succeed, else rolls back and rethrows leaving old backend + `currentId`s intact. Atomic exactly when the new backend answers `kCallerMayBlock`; a `kCallerMustNotBlock` backend's binds are deferred and the switch is not all-or-nothing (see above). Cancels old backend's pending ops with `BackendChangedError`. Holds both `_mtx` and `_attachMtx` for its staging and commit, and resolves `whenBound()` waiters after releasing them. The `unique_ptr` overload is a template on the concrete backend type and delegates to the `shared_ptr` one — see below. | | `deregisterHandler` | `void deregisterHandler(const shared_ptr&)` | Deregisters from active backend (if bound), resets `currentId` to 0, removes from tracking. | -| `executeVia` | `Completion executeVia(const shared_ptr&, Action, IExecutor*)` | Lock-free dispatch. Attaches default session. On `LocalBackend`, rejects an action whose `ActionValidator::ready` returns `false` with `morph::model::ValidationError` via `onError`, before `Model::execute` runs. Records a journal `LogEntry` for loggable actions on both success (`Outcome::Succeeded`) and a throwing `Model::execute` (`Outcome::Failed`, rethrown unchanged). Dispatches through `IBackend::executeInto`, handing the backend a `detail::BridgeSink` that is simultaneously the caller's typed completion state and the backend's settle sink — one allocation where an erased-completion forwarding block costs six. Value-forwarding into the typed `Completion` is `try`/`catch`-guarded — a throwing result move/copy resolves the completion via `onError` instead of hanging or terminating. The bridge-touching side effects (`onResult`, `hasSubscribers()`/`publishResult`, the `pendingCalls()` decrement, and the execute-deadline disarm) are gated on the bridge's `CallbackToken`, checked before any runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. Increments `pendingCalls()` once per call before dispatch (never for the synchronous "handler not bound" early return); decrements it exactly once, from whichever of the two mutually-exclusive resolution continuations actually fires. Arms the client-side execute deadline when one is installed (see `setExecuteDeadline`); the fast-fail "handler not bound" path returns before that and arms nothing. | +| `executeVia` | `Completion executeVia(const shared_ptr&, Action, IExecutor*)` | Lock-free dispatch. Attaches default session. On `LocalBackend`, rejects an action whose `ActionValidator::ready` returns `false` with `morph::model::ValidationError` via `onError`, before `Model::execute` runs. Records a journal `LogEntry` for loggable actions on both success (`Outcome::Succeeded`) and a throwing `Model::execute` (`Outcome::Failed`, rethrown unchanged); a failure to serialise the result or to append the success entry happens after the mutation committed and rejects the completion with `morph::model::ActionRecordingError` instead of recording `Outcome::Failed`. Dispatches through `IBackend::executeInto`, handing the backend a `detail::BridgeSink` that is simultaneously the caller's typed completion state and the backend's settle sink — one allocation where an erased-completion forwarding block costs six. Value-forwarding into the typed `Completion` is `try`/`catch`-guarded — a throwing result move/copy resolves the completion via `onError` instead of hanging or terminating. The bridge-touching side effects (`onResult`, `hasSubscribers()`/`publishResult`, the `pendingCalls()` decrement, and the execute-deadline disarm) are gated on the bridge's `CallbackToken`, checked before any runs, so a completion resolving after `~Bridge()` skips them instead of touching the dangling `Bridge`. Increments `pendingCalls()` once per call before dispatch (never for the synchronous "handler not bound" early return); decrements it exactly once, from whichever of the two mutually-exclusive resolution continuations actually fires. Arms the client-side execute deadline when one is installed (see `setExecuteDeadline`); the fast-fail "handler not bound" path returns before that and arms nothing. | | `setDefaultSession` | `void setDefaultSession(session::Context)` | Installs default session context; also pushes it to the active backend via `IBackend::setSession` so control envelopes (register/attach/assign/deregister) carry it too, not only `execute`. | | `defaultSession` | `session::Context defaultSession() const` | Returns snapshot of default session. | | `setExecuteDeadline` | `void setExecuteDeadline(std::chrono::milliseconds)` | Opt-in client-side execute deadline; `0` (the default) disables it. Lazily creates the backing `TimeoutScheduler` thread on first enable. | diff --git a/docs/spec/core/registry.md b/docs/spec/core/registry.md index ae3d2e4b0..ee8c3c28b 100644 --- a/docs/spec/core/registry.md +++ b/docs/spec/core/registry.md @@ -55,6 +55,12 @@ provides: runner (`ActionDispatcher::registerAction`), and the local `Bridge::executeVia` path — the last two throw `ValidationError` on a `false` result instead of running `Model::execute`. +- **`ActionRecordingError`** — thrown when an action executed and its mutation + committed, but serialising the result or appending the journal entry failed. + Carries `cause()` (the underlying message) and `result()` (the committed + action's result JSON, or `""` when serialising it is what failed). Derives + from `std::runtime_error`, so existing `catch (const std::exception&)` paths + are unaffected. - **Logging policy** — `ActionLogPolicy` and `Loggable` that control whether an action's executions are recorded and how duplicates are coalesced. - **Type-erased holders** — `IModelHolder` / `ModelHolder` that own a model @@ -600,12 +606,18 @@ class ActionDispatcher { enforcement and before the validator check, so the validator sees the authoritative computed value), enforces `ActionValidator::ready(action)` (throwing `ValidationError` on `false`, before `Model::execute` runs), then calls `Model::execute(action)` - inside a `try`/`catch (const std::exception&)`: on success it serialises the - result and records a `LogEntry` with `outcome = Outcome::Succeeded` (when - loggable and a log is attached); on a throw it records `outcome = - Outcome::Failed` (`error = exc.what()`, `result` empty) for the same actions - and rethrows unchanged, so callers see the same exception as before — the - journal entry is a side effect, not a change to error propagation. Mirrors + inside a `try`/`catch (const std::exception&)`: on a throw it records + `outcome = Outcome::Failed` (`error = exc.what()`, `result` empty) when the + action is loggable and a log is attached, and rethrows unchanged, so callers + see the same exception as before — the journal entry is a side effect, not a + change to error propagation. `Model::execute` is the **only** call inside that + `try`: serialising the result and recording `outcome = Outcome::Succeeded` run + after it, outside, because by then the mutation has committed and a throw from + either is not an execution failure. Both surface as + `morph::model::ActionRecordingError` instead — see [journal.md, "A refused + recording is not an execution + failure"](../journal/journal.md#a-refused-recording-is-not-an-execution-failure). + Mirrors `Bridge::executeVia`'s `localOp` (`bridge.md`) for `LocalBackend`. See [journal.md, "Outcome"](../journal/journal.md#logentry--one-recorded-action-execution) for the full field/replay semantics. Every recorded entry — success or diff --git a/docs/spec/journal/journal.md b/docs/spec/journal/journal.md index 050b45025..8e4999455 100644 --- a/docs/spec/journal/journal.md +++ b/docs/spec/journal/journal.md @@ -27,6 +27,7 @@ by `contextKey`; see [Attaching a log to remote instances](#attaching-a-log-to-r ## Contents - [LogEntry — one recorded action execution](#logentry--one-recorded-action-execution) + - [A refused recording is not an execution failure](#a-refused-recording-is-not-an-execution-failure) - [Serialization](#serialization) - [Why the codec is a separate header](#why-the-codec-is-a-separate-header) - [Line-format version (`v`)](#line-format-version-v) @@ -84,6 +85,49 @@ the `catch` records a `Failed` entry (`error = exc.what()`, `result` empty) and rethrows unchanged, so the caller's error handling is unaffected — only the journal gains an entry it previously lacked. +### A refused recording is not an execution failure + +`Outcome::Failed` means the model rejected the action, and nothing else. Only +`Model::execute` can produce it, so it is the only call inside the `try` that +records one. + +Two steps run after it and before the caller is answered: serialising the +result (`ActionTraits::resultToJson`, which raises +`model::detail::ParseError` on a write error) and appending the `LogEntry`. Both +can throw, and by the time either does the model's mutation has already +committed — an `IActionLog` that could not reach its backend is *required* to +throw, since `append`/`flush` return `void` and that is the only channel the +interface gives it (`FileActionLog` throws from eighteen sites; a full disk or a +revoked permission reaches them in production). + +Inside the execution `try` they would give three wrong answers at once: a caller +told a durable write was rejected and free to retry it, an `Outcome::Failed` +entry in the audit trail for a mutation that committed, and that entry's `error` +carrying the *sink's* message, permanently blaming the action for an +infrastructure fault. An audit log that is wrong about which actions succeeded is +worse than one missing entries, because nothing downstream can tell the two apart. + +So both steps sit outside that `try`, and a throw from either surfaces as +`morph::model::ActionRecordingError` — a `std::runtime_error` subclass whose +`what()` is `"action executed but was not recorded: "`, with `cause()` +returning the underlying message and `result()` the committed action's result +JSON. The caller still learns the recording failed; what changed is what it is +told, which is now true: the write happened, the record of it did not. Existing +`catch (const std::exception&)` handling is unaffected — `RemoteServer` still +turns it into an `err` reply, `LocalBackend` still rejects the `Completion` +through `onError` — and a caller that wants to distinguish a +committed-but-unrecorded action from a rejected one catches the type. + +When the throw came from `resultToJson`, **no entry is written at all**. A +`Succeeded` entry carries the result by definition (see `LogEntry::result` +above), and there is none to carry; the caller is told, which is the only +channel left. A committed mutation with no entry is a gap, but a smaller one +than an entry asserting it failed. + +`OutboxRelay` is unaffected by any of this: it calls `sink->append()` and +`sink->flush()` directly, and depends on their throwing — an outbox row is marked +relayed only after the sink returned normally. + ## Serialization ### Why the codec is a separate header @@ -1307,6 +1351,13 @@ These hold for every sink and are relied on by `replay()`/`undoLast()`: success, so replaying `payload` re-derives an equivalent `result` for a deterministic model. A `Failed` entry has no `result` to derive — see `replay()`, next. +- **A `Failed` entry means the model rejected the action.** It never means the + framework could not record a success. Serialising the result and appending the + entry run after the mutation has committed, outside the `try` that records + `Failed`; a throw from either is reported to the caller as + `morph::model::ActionRecordingError` and files no entry. See [A refused + recording is not an execution + failure](#a-refused-recording-is-not-an-execution-failure). - **`replay()`/`undoLast()` skip `Failed` entries.** A failed attempt never mutated model state, so there is nothing to reconstruct from it — and re-dispatching it would likely throw the same exception again, aborting diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index d79c3738c..d5c89d45e 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -2108,35 +2108,51 @@ class Bridge { // Local mode has no client/server split, so this is the same execution // site `ActionDispatcher::registerAction`'s runner is for remote modes // (registry.hpp) — see that overload's doc comment for the full story, - // including why both the success and failure paths below record a - // journal entry (a rejected/throwing execute must not leave the audit - // trail silent) and why the exception is rethrown unchanged either way. - try { - auto result = std::make_shared(model.execute(actionRef)); - if constexpr (::morph::model::detail::actionLoggable() == ::morph::model::Loggable::Yes) { - if (holder.hasActionLog()) { - // entityKey/principal/timestampMs are filled in by recordIfAttached. - ::morph::model::detail::recordActionSuccess( - holder, std::string{::morph::model::ModelTraits::typeId()}, - std::string{::morph::model::ActionTraits::typeId()}, - ::morph::model::ActionTraits::toJson(actionRef), - ::morph::model::detail::actionPayloadSchema(), - ::morph::model::ActionTraits::resultToJson(*result)); + // including why a rejected/throwing execute must not leave the audit + // trail silent, and why `Model::execute` is the only call inside the + // try that records Outcome::Failed. + auto result = [&] { + try { + return std::make_shared(model.execute(actionRef)); + } catch (const std::exception& exc [[maybe_unused]]) { + if constexpr (::morph::model::detail::actionLoggable() == ::morph::model::Loggable::Yes) { + if (holder.hasActionLog()) { + ::morph::model::detail::recordActionFailure( + holder, std::string{::morph::model::ModelTraits::typeId()}, + std::string{::morph::model::ActionTraits::typeId()}, + ::morph::model::ActionTraits::toJson(actionRef), + ::morph::model::detail::actionPayloadSchema(), exc.what()); + } } + throw; } - return result; - } catch (const std::exception& exc [[maybe_unused]]) { - if constexpr (::morph::model::detail::actionLoggable() == ::morph::model::Loggable::Yes) { - if (holder.hasActionLog()) { - ::morph::model::detail::recordActionFailure( + }(); + // Past this point the model's mutation has committed, so neither + // serialising the result nor appending the entry may be reported as + // an execution failure: both throw (ParseError; a sink that could + // not reach its backend), and inside the try above that throw would + // reject this call's Completion as if the model had refused the + // action and file an Outcome::Failed entry blaming the action for an + // infrastructure fault. ActionRecordingError says what is true + // instead -- the action ran, the recording of it did not -- and + // carries the result JSON the audit trail never received. + if constexpr (::morph::model::detail::actionLoggable() == ::morph::model::Loggable::Yes) { + if (holder.hasActionLog()) { + std::string resultJson; + try { + resultJson = ::morph::model::ActionTraits::resultToJson(*result); + // entityKey/principal/timestampMs are filled in by recordIfAttached. + ::morph::model::detail::recordActionSuccess( holder, std::string{::morph::model::ModelTraits::typeId()}, std::string{::morph::model::ActionTraits::typeId()}, ::morph::model::ActionTraits::toJson(actionRef), - ::morph::model::detail::actionPayloadSchema(), exc.what()); + ::morph::model::detail::actionPayloadSchema(), resultJson); + } catch (const std::exception& exc) { + throw ::morph::model::ActionRecordingError{std::move(resultJson), exc.what()}; } } - throw; } + return result; #endif }; { diff --git a/include/morph/core/registry.hpp b/include/morph/core/registry.hpp index 295028188..2c9164487 100644 --- a/include/morph/core/registry.hpp +++ b/include/morph/core/registry.hpp @@ -297,6 +297,63 @@ struct ValidationError : std::runtime_error { : std::runtime_error("action failed validation: " + std::string{modelType} + "/" + std::string{actionType}) {} }; +/// @brief Thrown when an action executed and its mutation committed, but the +/// framework could not finish reporting that success. +/// +/// Raised by the two sites that actually run `Model::execute` — the server +/// dispatch path (`morph::model::detail::ActionDispatcher::registerAction`'s +/// runner) and the in-process path (`Bridge::executeVia`'s `localOp`, +/// `bridge.hpp`) — for anything that fails *after* `Model::execute` returned: +/// serialising the result (`ActionTraits::resultToJson`, which throws +/// `detail::ParseError`) and appending the journal entry (a +/// `journal::IActionLog` whose `append` could not reach its backend, which +/// `action_log.hpp` requires it to signal by throwing). +/// +/// It exists because those failures are not execution failures and must not be +/// reported as one. An action whose journal append throws has already changed +/// the model; telling the caller the action was rejected invites a retry of a +/// write that landed, and recording `journal::Outcome::Failed` for it puts a +/// permanent lie in the audit trail, attributed to the action rather than to +/// the infrastructure that failed. So a post-execution failure is a distinct +/// type carrying a distinct message: the action ran, the reporting did not. +/// +/// No journal entry is written when the throw came from `resultToJson`. A +/// `Succeeded` entry carries the result by definition (see +/// `docs/spec/journal/journal.md`, `LogEntry::result`), and there is none to +/// carry; the caller is told instead, which is the only channel left. +/// +/// Deriving from `std::runtime_error` keeps existing `catch (const +/// std::exception&)` handling working — `RemoteServer::dispatchExecute`'s +/// strand catch still turns this into an `err` reply, and `LocalBackend`'s +/// still rejects the `Completion` — while a caller that wants to tell a +/// committed-but-unrecorded action from a rejected one can catch this type and +/// read `result()`. +class ActionRecordingError : public std::runtime_error { +public: + /// @brief Constructs the error with a message of the form + /// `"action executed but was not recorded: "`. + /// @param result JSON-encoded result of the committed action, or `""` when + /// serialising it is what failed. + /// @param cause `std::exception::what()` from the failure that stopped the + /// recording. + ActionRecordingError(std::string result, std::string cause) + : std::runtime_error("action executed but was not recorded: " + cause), + _result(std::move(result)), + _cause(std::move(cause)) {} + + /// @brief Returns the committed action's JSON-encoded result. + /// @return The result JSON, or `""` when `resultToJson` is what threw. + [[nodiscard]] const std::string& result() const noexcept { return _result; } + + /// @brief Returns the underlying failure's message, without this type's prefix. + /// @return `std::exception::what()` of the failure that stopped the recording. + [[nodiscard]] const std::string& cause() const noexcept { return _cause; } + +private: + std::string _result; + std::string _cause; +}; + /// @brief Whether an action's executions are recorded to an attached action log. /// /// A strong type instead of a bare `bool` so registration call sites read as @@ -612,8 +669,17 @@ class ActionDispatcher { /// successful `Model::execute` with `outcome = Outcome::Succeeded` and the /// JSON result, and equally on a thrown `std::exception` (a validation /// failure, a rejected write) with `outcome = Outcome::Failed` and - /// `error = what()`, `result` empty. Either way the exception (if any) - /// propagates unchanged after the entry is recorded. + /// `error = what()`, `result` empty. An exception from `Model::execute` + /// propagates unchanged after the `Failed` entry is recorded. + /// + /// `Outcome::Failed` means the model rejected the action, and nothing else: + /// only `Model::execute` can produce it. Serialising the result and + /// appending the entry both run after the model's mutation has committed, + /// and both can throw -- `resultToJson` raises `detail::ParseError`, and a + /// sink that could not reach its backend is required to say so by throwing + /// (`journal/action_log.hpp`). Those surface as `ActionRecordingError`, so + /// a committed write is never reported to the caller, or filed in the + /// audit trail, as a rejected one. /// /// Every recorded entry is stamped with `payloadFingerprint()` in /// `LogEntry::schema`, and the same fingerprint is filed under @@ -622,6 +688,8 @@ class ActionDispatcher { /// shape which wrote an entry is not the shape it is about to decode it /// with -- see `docs/spec/journal/journal.md`, "Payload schema fingerprint". /// @throws ValidationError if the decoded action fails `ActionValidator::ready`. + /// @throws ActionRecordingError if the action executed but its result could + /// not be serialised or its journal entry could not be appended. template void registerAction(std::string_view modelId, std::string_view actionId) { ActionEntry& entry = _actions[Key{std::string{modelId}, std::string{actionId}}]; @@ -665,15 +733,42 @@ class ActionDispatcher { throw ValidationError{ModelTraits::typeId(), ActionTraits::typeId()}; } auto& model = holder.template into(); - // Both the success and failure paths below record a journal entry - // (when a log is attached and Action is loggable) so a rejected or - // throwing execution -- a validation failure, a lost connection, a - // rejected write -- still leaves an audit trail, not silence. The - // exception is rethrown unchanged either way; only the outcome - // shape differs. See docs/spec/journal/journal.md, "Outcome". + // Model::execute is the only call inside this try, because it is + // the only one whose failure means the action was rejected. A + // rejected or throwing execution -- a validation failure, a lost + // connection, a rejected write -- records Outcome::Failed (when a + // log is attached and Action is loggable) so it leaves an audit + // trail rather than silence, and the exception is rethrown + // unchanged. See docs/spec/journal/journal.md, "Outcome". + auto result = [&] { + try { + return model.execute(action); + } catch (const std::exception& exc [[maybe_unused]]) { + if constexpr (detail::actionLoggable() == Loggable::Yes) { + if (holder.hasActionLog()) { + detail::recordActionFailure(holder, std::string{ModelTraits::typeId()}, + std::string{ActionTraits::typeId()}, + std::string{payloadJson}, + detail::actionPayloadSchema(), exc.what()); + } + } + throw; + } + }(); + // Past this point the model's mutation has committed, so neither + // step below may be reported as an execution failure. Both can + // still throw -- resultToJson raises ParseError, and a sink that + // could not reach its backend is required to throw (see + // journal/action_log.hpp) -- and inside the try above that throw + // would tell the caller a durable write was rejected and file an + // Outcome::Failed entry naming the infrastructure fault as the + // action's own error. ActionRecordingError instead says what is + // true: the action ran, the reporting of it did not. A throw from + // resultToJson leaves no entry at all, because a Succeeded entry + // carries the result and there is none to carry. + std::string resultJson; try { - auto result = model.execute(action); - auto resultJson = ActionTraits::resultToJson(result); + resultJson = ActionTraits::resultToJson(result); if constexpr (detail::actionLoggable() == Loggable::Yes) { if (holder.hasActionLog()) { // entityKey/principal/timestampMs are filled in by recordIfAttached. @@ -683,18 +778,10 @@ class ActionDispatcher { resultJson); } } - return resultJson; - } catch (const std::exception& exc [[maybe_unused]]) { - if constexpr (detail::actionLoggable() == Loggable::Yes) { - if (holder.hasActionLog()) { - detail::recordActionFailure(holder, std::string{ModelTraits::typeId()}, - std::string{ActionTraits::typeId()}, - std::string{payloadJson}, detail::actionPayloadSchema(), - exc.what()); - } - } - throw; + } catch (const std::exception& exc) { + throw ActionRecordingError{std::move(resultJson), exc.what()}; } + return resultJson; }; entry.coalesce = ActionLogPolicy::coalesce; entry.schema = detail::actionPayloadSchema(); diff --git a/tests/test_action_log.cpp b/tests/test_action_log.cpp index 7749b649f..3eeef63de 100644 --- a/tests/test_action_log.cpp +++ b/tests/test_action_log.cpp @@ -142,6 +142,133 @@ struct morph::model::ModelTraits { static constexpr std::string_view typeId() { return "AL_LegacyModel"; } }; +// A result this build cannot serialise: resultToJson throws where a real one +// raises ParseError on a glaze write error. Serialisation runs after the model +// has already mutated, so it is the same post-commit failure as a refused +// journal append, with no journal involved. +struct ALUnserialisableAction { + int amount = 0; +}; +struct ALUnserialisableModel { + int balance = 0; + int execute(const ALUnserialisableAction& a) { + balance += a.amount; + return balance; + } +}; + +template <> +struct morph::model::ActionTraits { + using Result = int; + static constexpr std::string_view typeId() { return "AL_Unserialisable"; } + static std::string toJson(const ALUnserialisableAction& a) { + return R"({"amount":)" + std::to_string(a.amount) + "}"; + } + static ALUnserialisableAction fromJson(std::string_view json) { + ALUnserialisableAction action{}; + auto pos = json.find(':'); + if (pos != std::string_view::npos) { + action.amount = std::stoi(std::string{json.substr(pos + 1)}); + } + return action; + } + static std::string resultToJson(const int& /*r*/) { + throw morph::model::detail::ParseError{"result will not serialise"}; + } + static int resultFromJson(std::string_view s) { return std::stoi(std::string{s}); } +}; +template <> +struct morph::model::ModelTraits { + static constexpr std::string_view typeId() { return "AL_UnserialisableModel"; } +}; + +// A sink that refuses to record a success. +// +// `IActionLog::append` must throw when the entry did not reach the backend -- +// the return type is `void`, so there is no other channel -- and `FileActionLog` +// does exactly that from eighteen sites; a full disk or a revoked permission +// reaches it in production. The refusal is aimed at `Outcome::Succeeded` only, so +// one sink proves both halves at once: that a committed action is not misreported +// as rejected, and that a genuinely rejected one still records `Outcome::Failed`. +namespace { +class SuccessRefusingLog : public IActionLog { +public: + void append(LogEntry entry) override { + std::scoped_lock const lock{_mtx}; + _offered.push_back(entry); + if (entry.outcome == morph::journal::Outcome::Succeeded) { + throw std::runtime_error("journal sink unavailable"); + } + entry.seq = ++_seq; + _stored.push_back(std::move(entry)); + } + + void flush() override {} + + [[nodiscard]] std::vector entries(std::string_view entityKey = {}) const override { + std::scoped_lock const lock{_mtx}; + if (entityKey.empty()) { + return _stored; + } + std::vector out; + for (const auto& entry : _stored) { + if (entry.entityKey == entityKey) { + out.push_back(entry); + } + } + return out; + } + + /// Every entry the framework asked this sink to record, including the ones it + /// refused -- the stronger of the two views, because it also catches a Failed + /// entry that was offered and then dropped rather than never written. + [[nodiscard]] std::vector offered() const { + std::scoped_lock const lock{_mtx}; + return _offered; + } + +private: + mutable std::mutex _mtx; + std::vector _stored; + std::vector _offered; + std::uint64_t _seq = 0; +}; + +// What a caller actually learned from one execution: the dynamic type it saw, +// and the strings it could read off it. +struct Reported { + bool recordingError = false; + std::string what; + std::string cause; + std::string result; +}; + +Reported reportedFrom(const std::exception_ptr& eptr) { + Reported seen; + try { + std::rethrow_exception(eptr); + } catch (const morph::model::ActionRecordingError& err) { + seen.recordingError = true; + seen.what = err.what(); + seen.cause = err.cause(); + seen.result = err.result(); + } catch (const std::exception& exc) { + seen.what = exc.what(); + } + return seen; +} + +template +Reported reportedFromCall(Fn&& call) { + try { + std::forward(call)(); + } catch (...) { + return reportedFrom(std::current_exception()); + } + return Reported{}; +} +} // namespace + // A model that reads morph::journal::isReplaying() from inside execute() -- // the shape Phase 6's rules engine will use to suppress rule evaluation. struct RMModel { @@ -483,6 +610,185 @@ TEST_CASE("Bridge/LocalBackend: local-mode execution records outcome=Failed when REQUIRE(entries[0].error == "insufficient funds"); } +// ── A refused recording is not an execution failure ────────────────────────── + +TEST_CASE("ActionDispatcher: a sink that refuses the success append does not report the action as failed", + "[action_log][dispatch]") { + morph::model::detail::ActionDispatcher dispatcher; + morph::model::detail::ModelRegistryFactory registry; + registry.registerModel("AL_Model"); + dispatcher.registerAction("AL_Model", "AL_Deposit"); + + auto holder = registry.create("AL_Model"); + auto log = std::make_shared(); + holder->attachActionLog(log, "acct-sink-down"); + + auto depositJson = morph::model::ActionTraits::toJson(ALDeposit{.amount = 10}); + auto seen = reportedFromCall([&] { dispatcher.dispatch("AL_Model", "AL_Deposit", *holder, depositJson); }); + + // The mutation is durable. That was never in question -- what the caller is + // told about it is. + REQUIRE(holder->into().balance == 10); + + // 1. The caller learns the action ran and was not recorded, as a distinct + // type carrying the committed result -- not as the model refusing the + // write, and not as the sink's bare message. + REQUIRE(seen.recordingError); + REQUIRE(seen.what == "action executed but was not recorded: journal sink unavailable"); + REQUIRE(seen.cause == "journal sink unavailable"); + REQUIRE(seen.result == "10"); + + // 2. No Outcome::Failed entry exists for the committed mutation -- none + // stored, and none even offered to the sink. + REQUIRE(log->entries().empty()); + REQUIRE(log->offered().size() == 1); + REQUIRE(log->offered()[0].outcome == morph::journal::Outcome::Succeeded); + for (const auto& entry : log->offered()) { + REQUIRE(entry.outcome != morph::journal::Outcome::Failed); + } +} + +TEST_CASE("ActionDispatcher: a genuine Model::execute throw still records Outcome::Failed with the model's message", + "[action_log][dispatch]") { + morph::model::detail::ActionDispatcher dispatcher; + morph::model::detail::ModelRegistryFactory registry; + registry.registerModel("AL_Model"); + dispatcher.registerAction("AL_Model", "AL_Withdraw"); + + auto holder = registry.create("AL_Model"); + // The same refusing sink: it accepts Failed appends, so the only thing that + // can change this outcome is the framework deciding a rejected action is a + // recording problem. + auto log = std::make_shared(); + holder->attachActionLog(log, "acct-refusing"); + + auto withdrawJson = morph::model::ActionTraits::toJson(ALWithdraw{.amount = 50}); + auto seen = reportedFromCall([&] { dispatcher.dispatch("AL_Model", "AL_Withdraw", *holder, withdrawJson); }); + + // 3. The regression this fix could easily cause: a rejected action must + // still surface the model's own exception, unwrapped. + REQUIRE_FALSE(seen.recordingError); + REQUIRE(seen.what == "insufficient funds"); + + auto entries = log->entries(); + REQUIRE(entries.size() == 1); + REQUIRE(entries[0].outcome == morph::journal::Outcome::Failed); + REQUIRE(entries[0].error == "insufficient funds"); + REQUIRE(entries[0].result.empty()); +} + +TEST_CASE("ActionDispatcher: a result that will not serialise is not recorded as a rejected action", + "[action_log][dispatch]") { + morph::model::detail::ActionDispatcher dispatcher; + morph::model::detail::ModelRegistryFactory registry; + registry.registerModel("AL_UnserialisableModel"); + dispatcher.registerAction("AL_UnserialisableModel", + "AL_Unserialisable"); + + auto holder = registry.create("AL_UnserialisableModel"); + auto log = std::make_shared(); + holder->attachActionLog(log, "acct-unserialisable"); + + auto seen = reportedFromCall( + [&] { dispatcher.dispatch("AL_UnserialisableModel", "AL_Unserialisable", *holder, R"({"amount":7})"); }); + + REQUIRE(holder->into().balance == 7); + REQUIRE(seen.recordingError); + REQUIRE(seen.cause == "result will not serialise"); + // No result survived serialisation, so there is none to hand back and none + // to record: a Succeeded entry carries the result by definition, so the + // entry is omitted rather than written empty. + REQUIRE(seen.result.empty()); + REQUIRE(log->entries().empty()); +} + +TEST_CASE("Bridge/LocalBackend: a sink that refuses the success append does not report the action as failed", + "[action_log][bridge]") { + morph::exec::ThreadPoolExecutor pool{2}; + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + + auto log = std::make_shared(); + auto binding = std::make_shared(); + binding->typeId = "AL_Model"; + binding->modelFactory = [log] { + auto holder = morph::model::detail::ModelFactory::create(); + holder->attachActionLog(log, "acct-sink-down-local"); + return holder; + }; + morph::bridge::BridgeHandler handler{bridge, &cbExec, binding}; + + std::exception_ptr failure; + std::atomic settled{false}; + handler.execute(ALDeposit{.amount = 10}) + .then([&](int) { settled.store(true); }) + .onError([&](const std::exception_ptr& eptr) { + failure = eptr; + settled.store(true); + }); + REQUIRE(morph::testing::waitUntil([&] { return settled.load(); })); + REQUIRE(failure); + auto seen = reportedFrom(failure); + + REQUIRE(seen.recordingError); + REQUIRE(seen.what == "action executed but was not recorded: journal sink unavailable"); + REQUIRE(seen.cause == "journal sink unavailable"); + REQUIRE(seen.result == "10"); + + // The mutation committed: ALGetBalance is Loggable::No, so it reads the same + // instance back without the sink ever being asked. + std::atomic balance{-1}; + handler.execute(ALGetBalance{}).then([&](int v) { balance.store(v); }).onError([](const std::exception_ptr&) {}); + REQUIRE(morph::testing::waitUntil([&] { return balance.load() != -1; })); + REQUIRE(balance.load() == 10); + + REQUIRE(log->entries().empty()); + REQUIRE(log->offered().size() == 1); + for (const auto& entry : log->offered()) { + REQUIRE(entry.outcome != morph::journal::Outcome::Failed); + } +} + +TEST_CASE("Bridge/LocalBackend: a genuine Model::execute throw still records Outcome::Failed with the model's message", + "[action_log][bridge]") { + morph::exec::ThreadPoolExecutor pool{2}; + SyncExec cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + + auto log = std::make_shared(); + auto binding = std::make_shared(); + binding->typeId = "AL_Model"; + binding->modelFactory = [log] { + auto holder = morph::model::detail::ModelFactory::create(); + holder->attachActionLog(log, "acct-refusing-local"); + return holder; + }; + morph::bridge::BridgeHandler handler{bridge, &cbExec, binding}; + + std::exception_ptr failure; + std::atomic settled{false}; + handler.execute(ALWithdraw{.amount = 50}) + .then([&](int) { settled.store(true); }) + .onError([&](const std::exception_ptr& eptr) { + failure = eptr; + settled.store(true); + }); + REQUIRE(morph::testing::waitUntil([&] { return settled.load(); })); + REQUIRE(failure); + auto seen = reportedFrom(failure); + + REQUIRE_FALSE(seen.recordingError); + REQUIRE(seen.what == "insufficient funds"); + + auto entries = log->entries(); + REQUIRE(entries.size() == 1); + REQUIRE(entries[0].actionType == "AL_Withdraw"); + REQUIRE(entries[0].entityKey == "acct-refusing-local"); + REQUIRE(entries[0].outcome == morph::journal::Outcome::Failed); + REQUIRE(entries[0].error == "insufficient funds"); + REQUIRE(entries[0].result.empty()); +} + TEST_CASE("Bridge/LocalBackend: local-mode execution without an attached log does not crash", "[action_log][bridge]") { morph::exec::ThreadPoolExecutor pool{2}; SyncExec cbExec; From eba1c761a33f11e72414f27f49a5cf22bbc0cee0 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Thu, 24 Sep 2026 00:43:34 +0200 Subject: [PATCH 2/4] tests: suppress MSVC's unreachable-code warning where a handler never returns Moving the journal append out of the execution `try` put the model call behind `return model.execute(action);` inside an immediately-invoked lambda. Two test translation units register an action whose handler body is a bare `throw`, so for those instantiations the `return` really is unreachable and MSVC raises C4702 -- fatal under WarningsAsErrors. gcc and clang do not warn. The warning is correct, so it is suppressed rather than argued with, and suppressed at the two translation units that instantiate the never-returning handler rather than in the public headers that contain the statement: MSVC reports C4702 at the first instantiation point in the TU, not at the line inside the header, which is the same reason and the same placement the pastebin paste-model test already uses. Nothing about the dispatcher changes. Before this fix `execute` was called in statement context, so there was no `return` for MSVC to judge; the warning is new because the structure is, not because the behaviour is. Not verified locally: no MSVC is available here, so the placement follows the existing precedent and the compiler's own reported line rather than a reproduction. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- tests/test_shared_instances.cpp | 10 ++++++++++ tests/test_subscription.cpp | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/test_shared_instances.cpp b/tests/test_shared_instances.cpp index a90ea1081..2c92cd73e 100644 --- a/tests/test_shared_instances.cpp +++ b/tests/test_shared_instances.cpp @@ -12,6 +12,16 @@ // // See docs/planned/shared_model_instances.md. +// A model action handler in this file never returns -- its body is a bare +// `throw` -- so in that instantiation the dispatcher's `return model.execute(...)` +// is genuinely unreachable and MSVC says so. The warning is raised at the first +// instantiation point in this translation unit rather than inside the header +// that contains the statement, so it is suppressed here rather than there, and +// file-scoped because more than one case instantiates the same template. +#if defined(_MSC_VER) +#pragma warning(disable : 4702) +#endif + #include #include #include diff --git a/tests/test_subscription.cpp b/tests/test_subscription.cpp index 46d324ce3..9c4fa1282 100644 --- a/tests/test_subscription.cpp +++ b/tests/test_subscription.cpp @@ -13,6 +13,16 @@ // produce it, so adding an action that also yields an `R` never breaks an // existing subscriber. +// A model action handler in this file never returns -- its body is a bare +// `throw` -- so in that instantiation the dispatcher's `return model.execute(...)` +// is genuinely unreachable and MSVC says so. The warning is raised at the first +// instantiation point in this translation unit rather than inside the header +// that contains the statement, so it is suppressed here rather than there, and +// file-scoped because more than one case instantiates the same template. +#if defined(_MSC_VER) +#pragma warning(disable : 4702) +#endif + #include #include #include From ea4aac1a7c3e25d991b052ab24dd367ebceceaff Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Thu, 24 Sep 2026 04:43:37 +0200 Subject: [PATCH 3/4] core: suppress MSVC's unreachable-code warning at the statement, not per translation unit The previous attempt put the C4702 suppression in the two test files the compiler happened to name. That was the wrong placement and CI said so: the next run reported the same warning from `test_flows_apps.cpp` and `test_sections.cpp` instead. The warning is about a statement in the header, so it fires from whichever translation unit instantiates a handler that never returns, and suppressing per consumer is an open-ended obligation -- eleven test files already register an `execute` overload whose body is a bare `throw`, and every future one would join them. So the suppression now sits around the statement it is about, in `registry.hpp` and `bridge.hpp`, guarded on `_MSC_VER` and scoped with push/pop so it disables nothing else. gcc and clang do not warn here and are unaffected; a syntax-only compile of a test translation unit under clang 22 is clean. The warning is correct for the instantiation that provokes it -- when `Model::execute` never returns, the `return` really is unreachable -- and wrong as a verdict on the statement, which every other instantiation reaches. The comment says that rather than claiming the compiler is mistaken. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- include/morph/core/bridge.hpp | 13 +++++++++++++ include/morph/core/registry.hpp | 13 +++++++++++++ tests/test_shared_instances.cpp | 10 ---------- tests/test_subscription.cpp | 10 ---------- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index d5c89d45e..fcbb863e5 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -2111,6 +2111,16 @@ class Bridge { // including why a rejected/throwing execute must not leave the audit // trail silent, and why `Model::execute` is the only call inside the // try that records Outcome::Failed. +// MSVC's C4702 fires on the `return` below for any action whose handler never +// returns -- a test double whose body is a bare `throw`, for instance. The +// warning is correct for that instantiation and wrong as a verdict on this +// statement, which every other instantiation reaches. It is suppressed here +// rather than in each translation unit that instantiates such a handler, +// because the set of those is open-ended: eleven test files already qualify. +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4702) +#endif auto result = [&] { try { return std::make_shared(model.execute(actionRef)); @@ -2127,6 +2137,9 @@ class Bridge { throw; } }(); +#if defined(_MSC_VER) +#pragma warning(pop) +#endif // Past this point the model's mutation has committed, so neither // serialising the result nor appending the entry may be reported as // an execution failure: both throw (ParseError; a sink that could diff --git a/include/morph/core/registry.hpp b/include/morph/core/registry.hpp index 2c9164487..58b7c11af 100644 --- a/include/morph/core/registry.hpp +++ b/include/morph/core/registry.hpp @@ -740,6 +740,16 @@ class ActionDispatcher { // log is attached and Action is loggable) so it leaves an audit // trail rather than silence, and the exception is rethrown // unchanged. See docs/spec/journal/journal.md, "Outcome". +// MSVC's C4702 fires on the `return` below for any action whose handler never +// returns -- a test double whose body is a bare `throw`, for instance. The +// warning is correct for that instantiation and wrong as a verdict on this +// statement, which every other instantiation reaches. It is suppressed here +// rather than in each translation unit that instantiates such a handler, +// because the set of those is open-ended: eleven test files already qualify. +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4702) +#endif auto result = [&] { try { return model.execute(action); @@ -755,6 +765,9 @@ class ActionDispatcher { throw; } }(); +#if defined(_MSC_VER) +#pragma warning(pop) +#endif // Past this point the model's mutation has committed, so neither // step below may be reported as an execution failure. Both can // still throw -- resultToJson raises ParseError, and a sink that diff --git a/tests/test_shared_instances.cpp b/tests/test_shared_instances.cpp index 2c92cd73e..a90ea1081 100644 --- a/tests/test_shared_instances.cpp +++ b/tests/test_shared_instances.cpp @@ -12,16 +12,6 @@ // // See docs/planned/shared_model_instances.md. -// A model action handler in this file never returns -- its body is a bare -// `throw` -- so in that instantiation the dispatcher's `return model.execute(...)` -// is genuinely unreachable and MSVC says so. The warning is raised at the first -// instantiation point in this translation unit rather than inside the header -// that contains the statement, so it is suppressed here rather than there, and -// file-scoped because more than one case instantiates the same template. -#if defined(_MSC_VER) -#pragma warning(disable : 4702) -#endif - #include #include #include diff --git a/tests/test_subscription.cpp b/tests/test_subscription.cpp index 9c4fa1282..46d324ce3 100644 --- a/tests/test_subscription.cpp +++ b/tests/test_subscription.cpp @@ -13,16 +13,6 @@ // produce it, so adding an action that also yields an `R` never breaks an // existing subscriber. -// A model action handler in this file never returns -- its body is a bare -// `throw` -- so in that instantiation the dispatcher's `return model.execute(...)` -// is genuinely unreachable and MSVC says so. The warning is raised at the first -// instantiation point in this translation unit rather than inside the header -// that contains the statement, so it is suppressed here rather than there, and -// file-scoped because more than one case instantiates the same template. -#if defined(_MSC_VER) -#pragma warning(disable : 4702) -#endif - #include #include #include From 6214c065d02a2ea937743e8c2e6745edf1dbd6a1 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Fri, 25 Sep 2026 09:02:13 +0200 Subject: [PATCH 4/4] core: guard the MSVC warning pragmas with #ifdef, as clang-tidy's readability-use-concise-preprocessor-directives requires Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Y4eif7wQNNhSkHUKYqq5Xq --- include/morph/core/bridge.hpp | 4 ++-- include/morph/core/registry.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/morph/core/bridge.hpp b/include/morph/core/bridge.hpp index fcbb863e5..b94dab001 100644 --- a/include/morph/core/bridge.hpp +++ b/include/morph/core/bridge.hpp @@ -2117,7 +2117,7 @@ class Bridge { // statement, which every other instantiation reaches. It is suppressed here // rather than in each translation unit that instantiates such a handler, // because the set of those is open-ended: eleven test files already qualify. -#if defined(_MSC_VER) +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable : 4702) #endif @@ -2137,7 +2137,7 @@ class Bridge { throw; } }(); -#if defined(_MSC_VER) +#ifdef _MSC_VER #pragma warning(pop) #endif // Past this point the model's mutation has committed, so neither diff --git a/include/morph/core/registry.hpp b/include/morph/core/registry.hpp index 58b7c11af..9afbfb3c6 100644 --- a/include/morph/core/registry.hpp +++ b/include/morph/core/registry.hpp @@ -746,7 +746,7 @@ class ActionDispatcher { // statement, which every other instantiation reaches. It is suppressed here // rather than in each translation unit that instantiates such a handler, // because the set of those is open-ended: eleven test files already qualify. -#if defined(_MSC_VER) +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable : 4702) #endif @@ -765,7 +765,7 @@ class ActionDispatcher { throw; } }(); -#if defined(_MSC_VER) +#ifdef _MSC_VER #pragma warning(pop) #endif // Past this point the model's mutation has committed, so neither