From ed2e91400c92e62d187b98db7682658f3261eafc Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Fri, 24 Apr 2026 14:29:35 +0100 Subject: [PATCH] Update documentation for ML-21, ML-9, and Erlang 28.5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - architecture.md: document new ErrorResponse behaviour, HttpError, and Worker.ErrorHandler modules plus per-API ErrorResponse pattern - project-conventions.md: capture API-failure → Oban tuple convention and mix_audit CI/pre-commit run - production-infrastructure.md: bump builder image Erlang to 28.5 and note mix_audit in the lint step --- docs/architecture.md | 11 +++++++++++ docs/production-infrastructure.md | 4 ++-- docs/project-conventions.md | 2 ++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index f18789c8..267cbf87 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -136,6 +136,9 @@ Last.fm schemas (separate, not Ecto-persisted to main DB): | `ErrorTracker.ErrorNotifier` | GenServer: attaches to ErrorTracker telemetry, skips muted errors, throttles repeated errors, dispatches email notifications | | `ErrorTracker.ErrorNotifier.Email` | Builds and sends Swoosh error notification emails with stack trace formatting | | `ErrorIgnorer` | ErrorTracker.Ignorer implementation: filters non-actionable errors (e.g., NoRouteError from bot scanners) | +| `MusicLibrary.ErrorResponse` | Behaviour for structured API error responses (`retryable?/1`, `retry_delay_seconds/1`) — per-API `ErrorResponse` modules implement this | +| `MusicLibrary.HttpError` | Default HTTP status → kind mapping (`:rate_limit`, `:server_error`, `:timeout`, `:auth_error`, `:not_found`, `:client_error`, `:unknown`) used as baseline by per-API `ErrorResponse` modules | +| `MusicLibrary.Worker.ErrorHandler` | Translates per-API `ErrorResponse` structs into Oban tuples — `{:snooze, seconds}` for retryable, `{:cancel, reason}` for permanent | | `MusicLibraryWeb.RecordsOnThisDayEmail` | Builds and sends daily "records on this day" email with cover images, anniversary styling | | `MusicLibrary.Mailer` | Swoosh mailer (Mailgun in prod, local adapter in dev) | | `FormatNumber` | Number formatting utility | @@ -159,6 +162,14 @@ Each has a `Config` module reading from application env. All HTTP clients use `R per-API rate limiting (`Req.RateLimiter`, ETS-backed). In tests, all HTTP calls are stubbed via `Req.Test` (configured in `config/test.exs`). +Each API also has an `API.ErrorResponse` module (e.g. `MusicBrainz.API.ErrorResponse`, +`OpenAI.API.ErrorResponse`) implementing the `MusicLibrary.ErrorResponse` behaviour, +so workers can uniformly classify HTTP failures as transient (snooze) or permanent +(cancel) via `MusicLibrary.Worker.ErrorHandler`. Per-API overrides capture +API-specific quirks — e.g. MusicBrainz uses HTTP 503 as the rate-limit signal, and +OpenAI splits HTTP 429 into `:rate_limit` vs `:auth_error` by reading the body +`code` (`insufficient_quota` → permanent). + --- ## Oban Workers (lib/music_library/worker/) diff --git a/docs/production-infrastructure.md b/docs/production-infrastructure.md index db088aa8..3d5d8ff9 100644 --- a/docs/production-infrastructure.md +++ b/docs/production-infrastructure.md @@ -25,7 +25,7 @@ push via GitHub Actions. The Docker image is a multi-stage build: -1. **Builder** — `hexpm/elixir:1.20.0-rc.4-erlang-28.4.3-debian-trixie-20260421-slim` with +1. **Builder** — `hexpm/elixir:1.20.0-rc.4-erlang-28.5-debian-trixie-20260421-slim` with Node.js 25, compiles deps, builds assets (`mix assets.deploy`), generates an OTP release. 2. **Runner** — `debian:trixie-20260421-slim` with minimal runtime deps (`libstdc++6`, `openssl`, `libncurses6`, `ca-certificates`). Runs as unprivileged `nobody` user. @@ -94,7 +94,7 @@ Uses `mise` (via `jdx/mise-action@v4`) for tool version management. ``` Push to main (or PR / manual dispatch) - ├── Lint (format, gettext, credo, sobelow, shellcheck, docker image, asset build) + ├── Lint (format, gettext, credo, sobelow, mix_audit, shellcheck, docker image, asset build) ├── Test (mix test with partitioning, coverage ≥75%) └── Deploy (requires GitHub environment approval, main branch only) ├── Trigger deployment via Coolify API (hurl) diff --git a/docs/project-conventions.md b/docs/project-conventions.md index e55c2de3..b0e6ade6 100644 --- a/docs/project-conventions.md +++ b/docs/project-conventions.md @@ -77,6 +77,7 @@ Rules extracted from commit history that are specific to this project and not al - **Non-actionable errors use `ErrorTracker.Ignorer`** behaviour to filter (e.g., NoRouteError from bot scanners) rather than blocking paths in the endpoint. - **Muted errors skip notifications.** `ErrorTracker.ErrorNotifier` checks the `muted` flag before sending email notifications. - **Oban worker return values follow three states:** `:ok` for success; `{:error, reason}` for transient/retryable failures (Oban will retry); `{:cancel, reason}` (not the deprecated `{:discard, reason}`) for permanent termination when the job outcome is non-retryable (e.g., no Wikipedia entry exists). +- **API failures flow through per-API `ErrorResponse` structs.** Each external API has a `.API.ErrorResponse` struct implementing the `MusicLibrary.ErrorResponse` behaviour (`retryable?/1`, `retry_delay_seconds/1`). Workers match app-layer atom-cancel reasons first (e.g. `:no_english_wikipedia`, `:cover_not_available`), then forward the remaining `{:error, _}` to `MusicLibrary.Worker.ErrorHandler.to_oban_result/1`, which emits `{:snooze, seconds}` for transient errors and `{:cancel, reason}` for permanent ones. - **Non-fatal enrichment failures are best-effort.** Context functions that enrich records (colors, embeddings) use a private `best_effort_*` helper that logs a warning and returns the unchanged struct rather than surfacing `{:error, reason}` to callers. Business-logic failures (e.g., API calls in `populate_genres/1`) still return `{:error, reason}`. ## Testing @@ -106,6 +107,7 @@ Rules extracted from commit history that are specific to this project and not al - **Alias nested modules at the top** rather than referencing them inline (e.g. `alias LastFm.Fixtures.RecentTracks` then `RecentTracks.get()`, not `LastFm.Fixtures.RecentTracks.get()`). Enforced by Credo's `AliasUsage` check. - **Markdown sanitization via MDEx (ammonia).** Use `Markdown.to_html/1` for user content. Annotate raw output with `# sobelow_skip ["XSS.Raw"]` and a comment explaining the sanitization. - **Sobelow runs on CI and pre-commit** in skip mode for security analysis. +- **`mix deps.audit` runs on CI and pre-commit** (warn-only) via `mix_audit` for CVE scanning. - **ExSlop checks run via Credo** for code quality: no narrator/boilerplate docs (use `@moduledoc false` instead), no obvious/step/narrator comments, no identity `case`/`with` patterns, no `Repo.all` then filter, no `Enum.map` with inline queries, no unaliased nested module use (nested modules must be aliased at the top). Violations are caught by CI. - **All modules require `@moduledoc`.** The Credo `ModuleDoc` check is enforced in strict mode. - **Dialyzer is enabled via `dialyxir` (dev/test only) with the `:no_opaque` flag.** Specs are required and checked. Opaque type checking is disabled to tolerate third-party opaque types (e.g., `Ecto.Changeset`).