The gap
morph::log's logging calls can throw, and docs/spec/core/logger.md documents this as deliberate:
A throwing sink propagates. Exceptions from the sink are not caught. They unwind out of detail::log […] and out of the originating logDebug/logInfo/… call into the caller. The logging layer adds no try/catch; a sink that must not disrupt its caller has to swallow its own exceptions internally.
That contract puts the burden on the sink. The problem is that a caller cannot rely on it: whether logging throws depends on which sink some other part of the program installed, and a library function in a noexcept context has no way to know or control that.
It is also not only the sink. detail::log takes a std::scoped_lock on LogState::mtx, and std::mutex::lock can throw std::system_error — so even with a perfectly well-behaved sink, no logging call is noexcept today.
This is already a live hazard, already worked around
CompletionState's destructor (include/morph/core/completion.hpp:149) logs orphaned exceptions. A destructor is implicitly noexcept, so a throwing logger there means std::terminate. The code already knows:
// NOLINTBEGIN(bugprone-empty-catch) — logError may throw; we swallow to avoid noexcept-escape
try {
std::rethrow_exception(error);
} catch (const std::exception& exc) {
try {
::morph::log::logError("[orphan] unhandled exception: " + std::string{exc.what()});
} catch (...) {
}
}
A nested try/catch(...) around a log call, plus a bugprone-empty-catch suppression, purely because the logging layer offers no guarantee. That is the workaround this issue proposes to make unnecessary.
Why it matters beyond that one site
The same shape recurs wherever a noexcept function wants to log. In #153 (Rational's saturating arithmetic) the operators must log on overflow; keeping them noexcept requires the identical local try/catch, or dropping noexcept from arithmetic operators that had it. Neither is a good trade for "we wanted to write a log line".
This is the same class of problem as #138 (every caller hand-rolls a liveness token) and #96/#97 (every caller needs its own fault seam): a guarantee that belongs in one place, currently reimplemented per call site — and easy to forget, because forgetting compiles.
Suggested direction
Make the logging layer non-throwing, and say so in the type system:
detail::log(LogLevel, std::string_view) noexcept — wrap the lock acquisition and the sink invocation in try { … } catch (...) {}.
- The public helpers (
logDebug/logInfo/logWarn/logError, both the string_view and variadic overloads) become noexcept too. The variadic ones must also guard std::format, which can throw std::bad_alloc.
- Update
docs/spec/core/logger.md's "Failure modes" section, replacing the "a throwing sink propagates" clause.
- Drop the local workaround in
CompletionState's destructor, and its NOLINT.
The trade-off worth stating explicitly: swallowing sink exceptions hides bugs in the sink. That is a real cost. But the alternative is that a logging call can take down an unrelated caller — or std::terminate a process from a destructor — and the spec already constrains sinks heavily (it tells them not to re-enter morph::log, and not to block). Treating "the sink misbehaved" as a logging-layer concern rather than a caller concern is consistent with that. If swallowing silently is unpalatable, a counter counting dropped log records would preserve the signal without propagating.
Alternative, if the current contract is intentional and should stand
If sink-propagation is genuinely wanted, then the honest fix is the opposite: document that morph::log must never be called from a noexcept context or a destructor, and audit the existing call sites for violations. CompletionState's destructor is one today. I do not think that is the better option — logging from teardown paths is exactly where it is most useful — but it is a coherent one, and the current state is neither.
Context
Found while implementing #153, where Rational's arithmetic operators need to log on overflow and would otherwise lose noexcept. That PR works around it locally the same way CompletionState does; the workaround can be removed if this lands.
The gap
morph::log's logging calls can throw, anddocs/spec/core/logger.mddocuments this as deliberate:That contract puts the burden on the sink. The problem is that a caller cannot rely on it: whether logging throws depends on which sink some other part of the program installed, and a library function in a
noexceptcontext has no way to know or control that.It is also not only the sink.
detail::logtakes astd::scoped_lockonLogState::mtx, andstd::mutex::lockcan throwstd::system_error— so even with a perfectly well-behaved sink, no logging call isnoexcepttoday.This is already a live hazard, already worked around
CompletionState's destructor (include/morph/core/completion.hpp:149) logs orphaned exceptions. A destructor is implicitlynoexcept, so a throwing logger there meansstd::terminate. The code already knows:A nested
try/catch(...)around a log call, plus abugprone-empty-catchsuppression, purely because the logging layer offers no guarantee. That is the workaround this issue proposes to make unnecessary.Why it matters beyond that one site
The same shape recurs wherever a
noexceptfunction wants to log. In #153 (Rational's saturating arithmetic) the operators must log on overflow; keeping themnoexceptrequires the identical localtry/catch, or droppingnoexceptfrom arithmetic operators that had it. Neither is a good trade for "we wanted to write a log line".This is the same class of problem as #138 (every caller hand-rolls a liveness token) and #96/#97 (every caller needs its own fault seam): a guarantee that belongs in one place, currently reimplemented per call site — and easy to forget, because forgetting compiles.
Suggested direction
Make the logging layer non-throwing, and say so in the type system:
detail::log(LogLevel, std::string_view) noexcept— wrap the lock acquisition and the sink invocation intry { … } catch (...) {}.logDebug/logInfo/logWarn/logError, both thestring_viewand variadic overloads) becomenoexcepttoo. The variadic ones must also guardstd::format, which can throwstd::bad_alloc.docs/spec/core/logger.md's "Failure modes" section, replacing the "a throwing sink propagates" clause.CompletionState's destructor, and itsNOLINT.The trade-off worth stating explicitly: swallowing sink exceptions hides bugs in the sink. That is a real cost. But the alternative is that a logging call can take down an unrelated caller — or
std::terminatea process from a destructor — and the spec already constrains sinks heavily (it tells them not to re-entermorph::log, and not to block). Treating "the sink misbehaved" as a logging-layer concern rather than a caller concern is consistent with that. If swallowing silently is unpalatable, a counter counting dropped log records would preserve the signal without propagating.Alternative, if the current contract is intentional and should stand
If sink-propagation is genuinely wanted, then the honest fix is the opposite: document that
morph::logmust never be called from anoexceptcontext or a destructor, and audit the existing call sites for violations.CompletionState's destructor is one today. I do not think that is the better option — logging from teardown paths is exactly where it is most useful — but it is a coherent one, and the current state is neither.Context
Found while implementing #153, where
Rational's arithmetic operators need to log on overflow and would otherwise losenoexcept. That PR works around it locally the same wayCompletionStatedoes; the workaround can be removed if this lands.