Some checks failed
CI / storybook-a11y (push) Successful in 4m12s
CI / backend (push) Successful in 1m6s
CI / api-client-drift (push) Successful in 1m37s
CI / frontend (push) Successful in 1m31s
CI / codeql (csharp) (push) Failing after 1m51s
CI / codeql (javascript-typescript) (push) Failing after 1m24s
Gap analysis found the POC's designed-but-unbuilt strategic gaps: ABAC authorization (ADR-0002/PRD-0002 phase P1), no e2e coverage, unproven i18n second-locale seam, thin resilience seams (correlation-id, idempotency, retry), and in-memory-only persistence. Each WP is grounded in the current code (file paths + line numbers), not just the analysis. Also corrects PRD-0001's stale 'Proposed' status header — the Mijn aanvragen vertical is fully built. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
133 lines
7.3 KiB
Markdown
133 lines
7.3 KiB
Markdown
# WP-21 — Resilience seams (correlation-id, idempotency, retry)
|
|
|
|
Status: todo
|
|
Phase: 5 — productie-volwassenheid
|
|
|
|
## Why
|
|
|
|
`api-client.provider.ts`'s own header comment lists four cross-cutting seams and
|
|
marks three "done" — but two of the three are only half-done, and the fourth
|
|
(retry/backoff) is an explicit unfilled seam:
|
|
|
|
- **Correlation id**: the FE generates a fresh `X-Correlation-Id` per request
|
|
(`api-client.provider.ts:29`), but the backend only _reads_ it opportunistically
|
|
inside the `Submit` helper (`Program.cs:344`) for log lines — there's no
|
|
middleware, so most endpoints never see or echo it, and it's never returned to
|
|
the caller for support/debugging correlation.
|
|
- **Idempotency key**: generated per-attempt (`api-client.provider.ts:31`), which
|
|
the same comment admits defeats its own purpose — "a real retry would thread a
|
|
STABLE key per logical submit so re-sends dedupe; here it's per-attempt." A retry
|
|
today would double-submit, not dedupe.
|
|
- **Retry/backoff**: not implemented at all — the comment names it as the one
|
|
remaining line to add, never added.
|
|
|
|
## Read first
|
|
|
|
- `src/app/shared/infrastructure/api-client.provider.ts` (the whole seam-comment
|
|
block at the top, lines 10-22, plus `httpClientFetch`'s header-building code)
|
|
- `backend/src/BigRegister.Api/Program.cs:344-368` (`Submit` helper — where
|
|
`X-Correlation-Id` is read today, and the only place)
|
|
- `backend/src/BigRegister.Api/Data/DocumentStore.cs:16` (`AuditEntry` — same
|
|
correlation id shape reused for audit `Actor` today, see `Program.cs:361` passing
|
|
`cid` as the audit actor for post-delivery)
|
|
|
|
## Decisions (pre-made, don't relitigate)
|
|
|
|
- **Correlation id becomes ASP.NET Core middleware**, not a per-endpoint read: every
|
|
request gets a correlation id (client-supplied `X-Correlation-Id` if present,
|
|
else server-generated), it's pushed into the logging scope for every log line in
|
|
that request (not just `Submit`'s), and echoed back as a response header so the
|
|
FE/caller can log it too.
|
|
- **Idempotency key becomes stable per logical operation**, generated once when a
|
|
submit/mutation _starts_ (e.g. once per wizard's submit action) and reused across
|
|
retries of that same logical attempt — not regenerated on every HTTP call. This
|
|
is a FE-side change (where the key is generated) plus a backend-side change
|
|
(actually deduping on it — see Files).
|
|
- **Retry/backoff applies only to idempotent GETs**, using rxjs `retry({ count,
|
|
delay })` in `httpClientFetch`'s pipe, per the existing header comment's own
|
|
suggestion. Writes are never auto-retried (the point of item above is making
|
|
retries _safe_, not making everything retry automatically — a POST retry policy
|
|
is a separate, larger decision about at-least-once semantics best left for when a
|
|
real backend needs it).
|
|
- Server-side idempotency _deduplication_ (actually short-circuiting a repeated key
|
|
to return the first result) is scoped to the submit endpoints only
|
|
(`Program.cs`'s `Submit` helper callers) — not every mutation — since that's
|
|
where the existing seam already concentrates correlation/idempotency handling.
|
|
|
|
## Files
|
|
|
|
- `backend/src/BigRegister.Api/Program.cs` — add correlation-id middleware
|
|
(`app.Use(async (ctx, next) => { … })` near the top of the pipeline, before route
|
|
registration): read-or-generate `X-Correlation-Id`, stash in
|
|
`ctx.Items`/`HttpContext`, push into `ILogger` scope
|
|
(`BeginScope(new Dictionary<string,object>{["CorrelationId"]=cid})`), set it on
|
|
`ctx.Response.Headers` before the response is written.
|
|
- `backend/src/BigRegister.Api/Program.cs` — simplify the `Submit` helper's own
|
|
`cid` read (now redundant with the middleware-populated value; read from
|
|
`HttpContext.Items` or inject via a lightweight accessor) so every log line in
|
|
`Submit` picks up the same id without re-parsing the header.
|
|
- New backend idempotency check: a small in-memory `IdempotencyStore` (same pattern
|
|
as `ApplicationStore`/`DocumentStore` — static dict + lock, ponytail-labeled with
|
|
the upgrade path to a real cache/store) keyed on `Idempotency-Key`, consulted by
|
|
the submit endpoints before calling `SubmissionRules.NewReference()`; returns the
|
|
cached response on a replayed key instead of minting a new reference.
|
|
- `src/app/shared/infrastructure/api-client.provider.ts` — generate the
|
|
`Idempotency-Key` once per logical submit rather than per HTTP attempt (thread it
|
|
in from the caller — likely means the submit commands in `application/submit-*.ts`
|
|
generate and pass the key, not the low-level fetch adapter); add
|
|
`retry({ count: 2, delay: 500 })` (or similar) to the GET-only path in the rxjs
|
|
pipe, gated on `method === 'GET'`.
|
|
- New backend test `backend/tests/BigRegister.Tests/IdempotencyTests.cs` — replay a
|
|
submit with the same `Idempotency-Key`, assert the same reference comes back and
|
|
`SubmissionRules.NewReference()` was not called twice (or assert the observable
|
|
effect: identical response body).
|
|
|
|
## Steps
|
|
|
|
1. Backend middleware for correlation id first (smallest, most mechanical change);
|
|
confirm every existing log line still works and now the id is consistent
|
|
end-to-end, not just inside `Submit`.
|
|
2. Backend `IdempotencyStore` + wiring into the submit endpoints; test the replay
|
|
behavior.
|
|
3. FE: move idempotency-key generation up to the command layer
|
|
(`submit-change-request.ts` and equivalents) so one logical submit = one key
|
|
even if `runSubmit`/the HTTP layer retries underneath.
|
|
4. FE: add GET retry/backoff in `httpClientFetch`; verify it doesn't retry writes
|
|
(assert via a spec on the adapter, or a targeted e2e/manual check with the
|
|
`?scenario=slow` toggle).
|
|
|
|
## Acceptance criteria
|
|
|
|
- [ ] Every backend log line for a given request shares one correlation id (not
|
|
just lines inside `Submit`); the id is echoed in the response headers.
|
|
- [ ] Replaying a submit with the same `Idempotency-Key` returns the same result
|
|
without minting a second reference (backend test proves this).
|
|
- [ ] A logical wizard submit generates exactly one `Idempotency-Key`, reused across
|
|
any FE-side retry of that submit (not regenerated per HTTP attempt).
|
|
- [ ] GET requests retry on transient failure (e.g. simulated via `?scenario=slow`
|
|
or a forced 5xx); POST/PUT/DELETE never auto-retry.
|
|
|
|
## Verification
|
|
|
|
GREEN + `cd backend && dotnet test`. Manual: `?scenario=error` on a GET-backed page,
|
|
confirm a retry attempt happens (network tab shows 2 requests) before the error
|
|
state renders; submit a wizard twice with a manually replayed idempotency key (e.g.
|
|
via curl against the backend directly) and confirm the second call doesn't create a
|
|
duplicate application.
|
|
|
|
## Out of scope
|
|
|
|
Retrying writes automatically (explicitly deferred, see Decisions); a durable
|
|
idempotency store surviving restart (in-memory is consistent with the rest of the
|
|
backend's persistence posture — see WP-22 if that changes); circuit breakers or
|
|
more advanced resilience patterns (Polly, etc.) — out of scope for a POC-scale
|
|
seam.
|
|
|
|
## Risks
|
|
|
|
Correlation-id middleware ordering matters — it must run before any endpoint that
|
|
logs, including error-handling middleware, or some log lines will still lack the
|
|
id. The idempotency store trades a small amount of memory for correctness under
|
|
replay; fine at demo scale, but the ponytail comment should name the real upgrade
|
|
(a TTL'd cache) so it isn't mistaken for a production-ready dedup mechanism.
|