Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.
Expand Down
34 changes: 30 additions & 4 deletions docs/go-cqrs.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -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`

Expand Down
Loading