Skip to content

Commit 9d8b9f8

Browse files
committed
docs(rfc): propose platform error classification
Summary: Define shared semantic error markers and platform-level retry classification, beginning with optimistic concurrency conflicts. Keep domain error identities while letting the generic classifier handle ErrVersionMismatch consistently across SubmitQueue and Stovepipe. Test Plan: - make fmt - make lint - make check-tidy - make check-gazelle Revert Plan: Revert this commit. API Changes: N/A Monitoring and Alerts: N/A
1 parent 39d61fe commit 9d8b9f8

2 files changed

Lines changed: 105 additions & 0 deletions

File tree

doc/rfc/error-classification.md

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# Platform Error Classification
2+
3+
Define shared semantic markers for errors whose default retry policy is stable across domains, beginning with optimistic concurrency conflicts.
4+
5+
## Problem
6+
7+
Queue error handling is already mostly centralized. Controllers return errors, `platform/errs` classifies them, and `platform/consumer` converts the resulting classification into ack, nack, or reject behavior. Platform classifiers already cover backend-independent failures such as cancellation and backend-specific failures such as transient MySQL errors.
8+
9+
The remaining gap is semantic errors defined by domain extensions. SubmitQueue and Stovepipe each expose `storage.ErrVersionMismatch`, but the platform cannot identify either sentinel without importing a domain package. That leaves three undesirable choices: every controller explicitly marks the error retryable, every domain introduces and wires its own classifier, or the error remains unclassified and is rejected instead of retried.
10+
11+
| Concern | Current behavior | Proposed behavior |
12+
|---|---|---|
13+
| Error meaning | Each domain defines an unrelated `ErrVersionMismatch` sentinel | Each sentinel also implements a shared optimistic-conflict marker |
14+
| Retry classification | Controllers or domain-specific classifiers must recognize the sentinel | The platform generic classifier maps the marker to `InfraRetryable` |
15+
| Controller responsibility | A controller may need to understand transport retry behavior | A controller handles only workflow-specific convergence and otherwise preserves the error chain |
16+
| Service wiring | A semantic rule may require another domain classifier | Primary consumers keep the standard platform generic and backend classifiers |
17+
18+
## Goals
19+
20+
- Define retry policy once when an error has the same recovery semantics across domains.
21+
- Keep domain error identities and existing `errors.Is` behavior.
22+
- Keep controllers responsible for workflow meaning, not nack and redelivery mechanics.
23+
- Preserve controller overrides when local state proves that a conflict is already resolved.
24+
- Avoid dependencies from `platform/` into SubmitQueue, Stovepipe, or future domains.
25+
26+
This RFC does not define queue retry limits or backoff, add automatic retries to RPC controllers, provide exactly-once execution, or create a shared storage interface.
27+
28+
## Proposal
29+
30+
Error handling is divided into three layers:
31+
32+
| Layer | Responsibility |
33+
|---|---|
34+
| Domain extension | Return a domain error that describes what happened |
35+
| Platform classifier | Map recognized error semantics to a `Verdict` |
36+
| Consumer and transport | Apply the verdict through ack, nack, reject, retry limits, and DLQ policy |
37+
38+
This separation puts retry classification in the common runtime because that runtime sees every returned error and already owns the transport action. Controllers remain free to interpret a conflict in workflow terms without duplicating transport policy.
39+
40+
### Optimistic-conflict marker
41+
42+
`platform/errs` defines a semantic marker:
43+
44+
```go
45+
type OptimisticConflict interface {
46+
error
47+
OptimisticConflict()
48+
}
49+
```
50+
51+
`OptimisticConflict` is a classification marker, not an error that controllers construct. A storage implementation detects a failed conditional write and returns its domain's `storage.ErrVersionMismatch`, whose concrete type implements the marker.
52+
53+
The marker describes the cause, not the action. It does not itself mean nack, choose a delay, or configure a retry limit. The platform classifier owns the policy that an unhandled optimistic conflict should be retried from fresh state.
54+
55+
SubmitQueue and Stovepipe retain their own `storage.ErrVersionMismatch` values. Storage callers can continue using `errors.Is(err, storage.ErrVersionMismatch)`, and platform code does not import either storage package.
56+
57+
### Generic classification
58+
59+
The existing `genericerrs.Classifier` recognizes the marker while continuing to classify backend-independent errors such as `context.Canceled`:
60+
61+
```text
62+
OptimisticConflict -> InfraRetryable
63+
context.Canceled -> InfraRetryable
64+
anything else -> Unknown
65+
```
66+
67+
Not every domain sentinel receives a platform classification. Errors such as `storage.ErrNotFound` and `storage.ErrAlreadyExists` remain context-specific because their meaning depends on the operation. Controllers classify them only when they have context unavailable to the platform classifiers.
68+
69+
The classifier examines one error-chain node at a time. `NewClassifierProcessor` remains responsible for walking wrapped errors, so domain and controller code must preserve causes with `%w`.
70+
71+
`mysqlerrs.Classifier` remains unchanged. It classifies raw MySQL driver, connection, and server errors; optimistic conflict is a semantic result produced by a conditional write and is not specific to MySQL.
72+
73+
The standard primary-consumer wiring remains entirely platform-owned:
74+
75+
```go
76+
errs.NewClassifierProcessor(
77+
genericerrs.Classifier,
78+
mysqlerrs.Classifier,
79+
)
80+
```
81+
82+
### Controller behavior
83+
84+
Controllers use their extension's documented sentinel, such as `storage.ErrVersionMismatch`, for workflow-specific handling. The `OptimisticConflict` marker is for cross-domain platform classification rather than normal controller matching.
85+
86+
When an optimistic conflict means another writer may have changed relevant state, the default controller behavior is to return the error unchanged apart from contextual wrapping:
87+
88+
```go
89+
if err := store.Update(...); err != nil {
90+
return fmt.Errorf("update batch %s: %w", batch.ID, err)
91+
}
92+
```
93+
94+
The classifier marks the conflict retryable, the consumer nacks the delivery, and redelivery reloads durable state before making another decision.
95+
96+
A controller may still handle the conflict locally when it has workflow knowledge the platform lacks. For example, if another writer already committed the desired terminal transition, the controller can acknowledge successful convergence:
97+
98+
```go
99+
if errors.Is(err, storage.ErrVersionMismatch) && desiredStateAlreadyCommitted {
100+
return nil
101+
}
102+
```
103+
104+
The platform rule is therefore a fallback for conflicts that escape the controller, not a requirement that every conflict produce redelivery. Existing local reconciliation loops may remain where they recompute from freshly loaded state and are part of the workflow operation rather than transport retry choreography.

doc/rfc/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting
88
- [Message Queue Contract](messagequeue-contract.md) - How queue payloads are defined (Protobuf, serialized as protobuf JSON), located by audience (external in `api/{domain}/messagequeue/`, internal in `{domain}/core/messagequeue/`), bound to topics (the `topics` proto option), and enforced by Bazel visibility
99
- [Consumer Gate](consumer-gate.md) - Stopping and starting individual queue controllers at runtime via consumer middleware: parked deliveries held in-flight with visibility extension, gate state as a separate extension with a file-based first implementation shared by tests and operators
1010
- [Change URIs](change-uri.md) - Identity of a code change: `scheme://{host[:port]}/{path}` per provider (GitHub PR, Phabricator Diff, git ref/commit) and canonical-form rules
11+
- [Platform Error Classification](error-classification.md) - Shared semantic error markers and platform retry classification, beginning with optimistic concurrency conflicts
1112

1213
## SubmitQueue
1314

0 commit comments

Comments
 (0)