112 lines
4.8 KiB
Markdown
112 lines
4.8 KiB
Markdown
# OpenSpec Change: logger-silent-failure-fix
|
|
|
|
- **Change ID**: logger-silent-failure-fix
|
|
- **Package**: `@suaveplan/logger`
|
|
- **Status**: PROPOSED
|
|
- **Date**: 2026-04-27
|
|
|
|
## Summary
|
|
|
|
Close two silent-failure bugs in `@suaveplan/logger` and tighten
|
|
`openspec/specs/logging/spec.md` to cover substrate behaviours
|
|
already shipped by the package: `MultiSink` fan-out semantics,
|
|
`EventLogger`, `StorageLogger`, `enable/disable`, `resetMetrics`,
|
|
and the `Sensitive<T>` redaction envelope. Pin every behavioural
|
|
fix and every previously-undocumented surface behind a
|
|
`*.spec-conformance.test.ts` so the 5-gate publish flow catches
|
|
drift.
|
|
|
|
## Current state
|
|
|
|
`@suaveplan/logger` is fully implemented and ships 171 passing
|
|
tests at 100% coverage, but two paths fail silently:
|
|
|
|
1. **Sink dispatch swallows everything.** `dispatchToSinks`
|
|
(`src/abstract/abstract-logger.ts`) catches every sink throw
|
|
and promise rejection and increments a single
|
|
`droppedByBackpressure++` counter. Spec defines a
|
|
`LOGGER.SINK` error code, but the dispatch path never raises
|
|
it — only `BufferedSink.flush` does. A serializer crash inside
|
|
a sink is misclassified as backpressure with no error surfaced
|
|
anywhere.
|
|
|
|
2. **`BufferedSink` block-policy deadlock.** Under
|
|
`policy: "block"`, `write()` parks writers in a private
|
|
`flushWaiters` list. Drain only happens when an external
|
|
caller invokes `flush()`. If nobody ever does, blocked writers
|
|
hang forever — there is no internal timer, no inline drain,
|
|
and no construction-time guard.
|
|
|
|
Additionally, requirements LG-11 / LG-12 / LG-13 don't reflect
|
|
the new failure-mode taxonomy and the spec lacks requirements
|
|
for `MultiSink`, the concrete loggers, lifecycle methods, and
|
|
`Sensitive<T>`.
|
|
|
|
## What changes
|
|
|
|
1. **Modify Requirement: LG-11 Metrics** — add
|
|
`droppedBySinkError: number` to the `LoggerMetrics` shape.
|
|
2. **Modify Requirement: LG-12 OTel instrumentation** — extend
|
|
the `logger.dropped_total` counter's `reason` attribute set
|
|
to include `"sink_error"`.
|
|
3. **Modify Requirement: LG-13 Structured errors** — codify
|
|
that `LOGGER.SINK` is raised by the dispatch path (not just
|
|
`BufferedSink.flush`) and routed to a configurable
|
|
`fallbackSink`.
|
|
4. **ADD Requirement: LG-21 Sink dispatch isolation** —
|
|
per-sink try/catch; one bad sink never halts the others;
|
|
each failure produces a `LoggerError(LOGGER.SINK)` written
|
|
to `fallbackSink` (default: a sink that calls
|
|
`console.error`).
|
|
5. **ADD Requirement: LG-22 BufferedSink no-deadlock contract**
|
|
— under `policy: "block"`, `write()` drains the head record
|
|
inline before pushing. No parked-writer list, no external
|
|
`flush()` required for forward progress.
|
|
6. **ADD Requirement: LG-23 MultiSink fan-out semantics** —
|
|
parallel dispatch via `Promise.all`; per-sink errors
|
|
isolated and reported to the parent logger's `fallbackSink`;
|
|
all sinks attempted regardless of order.
|
|
7. **ADD Requirement: LG-24 Generic Logger surface** —
|
|
`Logger<TMetadata>` is generic over metadata shape;
|
|
defaults to `Record<string, unknown>` for back-compat with
|
|
the original LG-02 `ctx: Record<string, unknown>` clause.
|
|
8. **ADD Requirement: LG-25 Concrete loggers** — `EventLogger`
|
|
and `StorageLogger` are documented exports with their
|
|
minimal contracts.
|
|
9. **ADD Requirement: LG-26 Lifecycle controls** — `enable()`,
|
|
`disable()`, `isEnabled()`, `resetMetrics()` are part of
|
|
the supported public surface.
|
|
10. **ADD Requirement: LG-27 Sensitive<T> envelope** —
|
|
`RedactionEngine` recognises `Sensitive`-tagged values and
|
|
redacts them automatically (LG-07 already names this; the
|
|
new requirement pins the verification path).
|
|
11. **Add `src/conformance.spec-conformance.test.ts`** pinning
|
|
LG-21, LG-22, and LG-23 so future drift exits the 5-gate
|
|
non-zero.
|
|
12. **Add `**Verified by:**` lines** to every requirement
|
|
(existing + new) in the folded spec.
|
|
|
|
## Why now
|
|
|
|
Both bugs are silent — the only signal a misbehaving sink emits
|
|
today is a generic counter increment with no error text, no
|
|
stack, no fallback record. The block-policy deadlock is
|
|
undetectable at construction time. Both ship green through the
|
|
existing 5-gate. Tightening the contract behind conformance
|
|
tests is the smallest possible change that prevents recurrence.
|
|
|
|
## Impact
|
|
|
|
- `@suaveplan/logger` — `LoggerMetrics` gains
|
|
`droppedBySinkError`. `AbstractLoggerConfig` gains
|
|
`fallbackSink?: Sink`. Two existing assertions in
|
|
`abstract-logger.test.ts` and one in
|
|
`abstract-logger.spec-conformance.test.ts` flip from
|
|
`droppedByBackpressure` / `reason: "backpressure"` to
|
|
`droppedBySinkError` / `reason: "sink_error"`. Coverage stays
|
|
at 100%.
|
|
- Downstream consumers — none affected; new metric and config
|
|
field are additive. `LOGGER.SINK` error code already exists.
|
|
- Spec readers — get an accurate picture of the dispatch
|
|
contract, the fallback sink, and the concrete loggers.
|