diff --git a/CHANGELOG.md b/CHANGELOG.md index 524b931..26568c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Please choose versions by [Semantic Versioning](http://semver.org/). * MINOR version when you add functionality in a backwards-compatible manner, and * PATCH version when you make backwards-compatible bug fixes. +## Unreleased + +- docs: clarify CQRS handler-error semantics in `go-cqrs.md` — handler errors do NOT cause kafka replay; the result-sender wrapper emits a single Failure result and commits the offset. Use `ErrCommandObjectSkipped` to suppress noisy Failure results when the caller condition is non-retryable. + ## v0.9.8 - chore: extract `check-versions` to `scripts/check-versions.sh`; add `make release-check` (`precommit + check-versions`); unwire `check-versions` from `precommit` so drift during development is allowed and alignment is enforced at release time. Add `docs/releasing-coding.md`. Aligns with `dark-factory` / `vault-cli` / `semantic-search` release-gate shape. diff --git a/docs/go-cqrs.md b/docs/go-cqrs.md index 49483fb..cdb9733 100644 --- a/docs/go-cqrs.md +++ b/docs/go-cqrs.md @@ -61,25 +61,51 @@ topic := schemaID.ResultTopic(branch) ## Skipping Invalid Commands -Return `cdb.ErrCommandObjectSkipped` when a command should be committed but not processed. Framework advances offset, sends no result. **Why:** `nil` silently swallows; normal error retries forever. +Return `cdb.ErrCommandObjectSkipped` when a command should be committed but not processed. Framework advances offset, sends no result. ```go // BAD — silently swallows, no visibility return nil, nil, nil -// BAD — framework sends failure result + retries +// BAD — emits a Failure on the result topic for every occurrence (noisy if caller is non-retryable) return nil, nil, err -// GOOD — skips with reason, no retry, no result +// GOOD — clean skip: no retry, no result emitted, offset advances return nil, nil, errors.Wrapf(ctx, cdb.ErrCommandObjectSkipped, "reason: %v", err) ``` **Use for:** malformed data, validation failure, duplicates, wrong state, filtered out. -**NOT for:** transient errors (network, disk) — return normal error so framework retries. +**NOT for:** transient errors (network, disk) — return normal error so the failure is visible on the result topic. + +## Handler Errors Do Not Cause Kafka Replay + +A common misconception: "If my handler returns `err`, kafka will replay the message forever." Not true for this framework. + +The result-sender wrapper catches the handler error, emits a `ResultObjectFailure` to the `*-result` topic, and returns `nil` to the outer kafka consumer. The offset commits on the next batch tick. Each error is **one** Failure on the result topic — not an infinite replay. + +``` +Handler returns err + ↓ +Wrapper sends ResultObjectFailure to *-result topic + ↓ +Wrapper returns nil to outer message handler + ↓ +Kafka offset commits → next message processed +``` + +In normal error-handling paths, the only case where offsets do NOT commit is when the result-sender itself fails to publish (e.g. kafka producer broken) — that bubbles a real error and triggers the kafka library's redelivery semantics. Process-level failures (panic escaping the wrapper, SIGKILL, OOM) also skip the commit, but those are infrastructure concerns, not application-level error handling. + +**Implications:** + +- Returning `err` from a non-retryable condition (wrong state, validation failure) is **functionally safe** — no replay loop — but it produces a `Failure` on the result topic for every occurrence. If a publisher emits N copies of the same command (no state pre-filter, broker confirm retries, etc.) you get N `Failure` entries and N error log lines. Use `ErrCommandObjectSkipped` to avoid that. +- Returning `err` from a **transient** condition (network blip, disk full) is still the right choice — but understand it produces a single Failure result and a single error log, NOT an automatic retry. If you want retry, build it into the handler or the orchestration around it. + +**Example pattern:** an order-processing handler returns an `InvalidStateError` whenever a command targets an order already in a terminal state (`Completed`, `Cancelled`). If the publisher does not pre-filter by state and emits N duplicate commands for the same order, the result topic gets N `Failure` entries and the log gets N error lines — none of them retries, all distinct messages. Fix: treat terminal states as an idempotent skip (`ErrCommandObjectSkipped`) rather than an error. ## Rules - Never consume event topic to wait for command results — use result topic - `RunCommandConsumerTx` wraps executors automatically — don't wrap manually - `ErrCommandObjectSkipped` skips silently (no result sent) — use for non-retryable situations +- Normal `err` returns are NOT retried by the framework; they emit one Failure result and commit the offset — same offset behaviour as Skipped, different result-topic behaviour - `SendResultEnabled() == false` + no error → no result sent - Context timeout → `ResultFor()` returns `Success: false`