From c91f3e754a854c3ded7a123789d4ee1145b8eb96 Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Wed, 5 Aug 2026 00:33:28 +0000 Subject: [PATCH] docs(P05): complete examples phase --- examples/bad/god-object.md | 107 +++++++++++++++++++++++ examples/bad/leaky-abstraction.md | 87 +++++++++++++++++++ examples/bad/silent-error.md | 88 +++++++++++++++++++ examples/good/api-endpoint.md | 79 +++++++++++++++++ examples/good/db-schema.md | 88 +++++++++++++++++++ examples/good/error-handler.md | 139 ++++++++++++++++++++++++++++++ examples/good/react-component.md | 108 +++++++++++++++++++++++ 7 files changed, 696 insertions(+) create mode 100644 examples/bad/god-object.md create mode 100644 examples/bad/leaky-abstraction.md create mode 100644 examples/bad/silent-error.md create mode 100644 examples/good/api-endpoint.md create mode 100644 examples/good/db-schema.md create mode 100644 examples/good/error-handler.md create mode 100644 examples/good/react-component.md diff --git a/examples/bad/god-object.md b/examples/bad/god-object.md new file mode 100644 index 0000000..2b77426 --- /dev/null +++ b/examples/bad/god-object.md @@ -0,0 +1,107 @@ +# Bad Example: God Object + +> A component that violates Atelier principles. Each violation is cited. + +## The Code + +```typescript +// UserManager.ts — 1,200 lines +class UserManager { + private users: User[] = []; + private cache: Map = new Map(); + private db: Database; + private emailService: EmailService; + private logger: Logger; + private auditLog: AuditLog; + + constructor(db: Database, email: EmailService, logger: Logger, audit: AuditLog) { + this.db = db; + this.emailService = email; + this.logger = logger; + this.auditLog = audit; + } + + // CRUD + async createUser(data: UserData): Promise { /* 80 lines */ } + async getUser(id: string): Promise { /* 40 lines */ } + async updateUser(id: string, data: Partial): Promise { /* 60 lines */ } + async deleteUser(id: string): Promise { /* 50 lines */ } + async listUsers(page: number): Promise { /* 40 lines */ } + + // Email + async sendWelcomeEmail(user: User): Promise { /* 50 lines */ } + async sendPasswordReset(user: User): Promise { /* 50 lines */ } + async sendDeletionNotice(user: User): Promise { /* 40 lines */ } + + // Auth + async authenticate(email: string, password: string): Promise { /* 70 lines */ } + async authorize(userId: string, action: string): Promise { /* 60 lines */ } + async hashPassword(password: string): Promise { /* 20 lines */ } + + // Cache + private cacheGet(id: string): User | null { /* 20 lines */ } + private cacheSet(user: User): void { /* 20 lines */ } + private cacheInvalidate(id: string): void { /* 20 lines */ } + + // Audit + private logAudit(action: string, userId: string): void { /* 30 lines */ } + + // Validation + private validateEmail(email: string): boolean { /* 20 lines */ } + private validatePassword(password: string): boolean { /* 20 lines */ } + + // Serialization + toJSON(user: User): Record { /* 30 lines */ } + fromJSON(data: Record): User { /* 30 lines */ } + + // ... 200 more lines of helper methods +} +``` + +## Violations + +### C3 Simplicity (Core) +- A 1,200-line class doing 8 different things (CRUD, email, auth, cache, audit, validation, serialization). +- The class cannot be understood in one read. Complexity is the liability. +- **Fix:** Split into `UserRepository` (CRUD), `UserEmailService` (email), `UserAuthService` (auth), `UserCache` (cache), `UserAuditLogger` (audit), `UserValidator` (validation), `UserSerializer` (serialization). + +### C6 Composability (Core) +- The class takes 4 dependencies and does 8 jobs. It is not composable; it is monolithic. +- You cannot reuse the email logic without the DB, the cache, the audit log. +- **Fix:** Each responsibility is its own class. Compose them: `UserEmailService` takes only `EmailService`. + +### components.md §1 Single Responsibility (UI/UX, applies to code) +- The class name is `UserManager`. "Manager" is a smell — it manages what? Everything. +- If the name is "Manager," it has no single responsibility. +- **Fix:** Name by responsibility: `UserRepository`, `UserAuthService`. Names that cannot be "And"-ed. + +### C4 Locality (Core) +- Cache logic is in the same class as email logic. A change to cache touches the email methods' neighbor. +- Related logic (cache get/set/invalidate) is grouped, but unrelated logic (email) is adjacent. +- **Fix:** `UserCache` is its own class. Cache changes are local to cache. + +### C2 Clarity (Core) +- A reader cannot answer "what does `UserManager` do?" in one sentence. +- The class has 20+ methods. The reader must scan all of them to find the one they need. +- **Fix:** Smaller classes with clear names. The name is the documentation. + +### Security P2 Least Privilege (Security) +- The class has `db`, `emailService`, `logger`, `auditLog` — all available to all methods. +- `sendWelcomeEmail` has access to `db.delete`. Least privilege is violated. +- **Fix:** Each service has only the dependencies it needs. `UserEmailService` has `EmailService`, not `Database`. + +### Testing P2 Independence (Testing) +- To test `sendWelcomeEmail`, you must construct `UserManager` with a real/mock DB, email, logger, audit. +- The test setup is 4 mocks for one method. Independence is violated. +- **Fix:** Test `UserEmailService` with one mock (`EmailService`). + +## What This Example Reveals + +The "God Object" is the cardinal sin of OOP. It violates C3 (Simplicity), C6 (Composability), C4 (Locality), and C2 (Clarity) simultaneously. Every other principle suffers downstream: + +- Testing is hard (T2 Independence). +- Security is loose (S2 Least Privilege). +- Evolution is brittle (a change to email risks cache). +- Review is exhausting (a 1,200-line diff). + +The fix is always the same: **decompose by responsibility**. The class name is the test: if it is "Manager," "Handler," or "Helper," it has no single responsibility. \ No newline at end of file diff --git a/examples/bad/leaky-abstraction.md b/examples/bad/leaky-abstraction.md new file mode 100644 index 0000000..8670489 --- /dev/null +++ b/examples/bad/leaky-abstraction.md @@ -0,0 +1,87 @@ +# Bad Example: Leaky Abstraction + +> An abstraction that leaks its implementation details, violating Atelier principles. Each violation is cited. + +## The Code + +```typescript +// UserRepository — "abstracts" the database +class UserRepository { + async findAll(): Promise { + // Leaks: returns the raw DB row type, not a domain User + return db.query('SELECT id, email, password_hash, created_at, deleted_at FROM users'); + } + + async findByEmail(email: string): Promise { + // Leaks: the caller must know to filter deleted_at + const rows = await db.query('SELECT * FROM users WHERE email = $1', [email]); + return rows[0] || null; + } + + async save(user: UserRow): Promise { + // Leaks: the caller must know the column names and the SQL + await db.query( + 'UPDATE users SET email = $1, password_hash = $2, updated_at = now() WHERE id = $3', + [user.email, user.password_hash, user.id] + ); + } +} + +// Usage — the leak is visible +const repo = new UserRepository(); +const user = await repo.findByEmail('jane@example.com'); +if (user && !user.deleted_at) { // caller must know about soft delete + user.password_hash = await hash(newPassword); // caller must know the column + await repo.save(user); // caller must know it's an UPDATE +} +``` + +## Violations + +### C6 Composability (Core) +- The abstraction is supposed to hide the database. It does not. +- The caller must know: the row type (`UserRow`), the soft-delete column (`deleted_at`), the password column (`password_hash`), the SQL operation (`UPDATE`). +- The abstraction is a thin wrapper. It composes nothing; it leaks everything. +- **Fix:** Return a domain `User` (no `password_hash`, no `deleted_at`). Hide soft delete (the repo filters it). Hide persistence (the caller calls `save`, not `UPDATE`). + +### C2 Clarity (Core) +- The caller's code is unclear: `if (user && !user.deleted_at)` — what is `deleted_at`? Why does the caller check it? +- The abstraction was supposed to clarify. It muddied. +- **Fix:** `repo.findByEmail()` returns `User | null` (already filtered). The caller does not know soft delete exists. + +### API P2 Clarity (API, by analogy) +- The repo's API exposes the DB schema in its return types. `UserRow` is a DB concept, not a domain concept. +- The public contract (return type) leaks the private implementation (the table). +- **Fix:** The return type is `User`, a domain type. `UserRow` is internal. + +### Data P8 Lifecycle Awareness (Data) +- The soft-delete lifecycle (`deleted_at`) is the repo's concern. The caller should not manage it. +- By exposing `deleted_at`, the repo forces every caller to remember the filter. A forgotten filter is a soft-delete leak. +- **Fix:** The repo filters `deleted_at IS NULL` in every query. The caller never sees `deleted_at`. + +### Security P9 Secret Hygiene (Security) +- `password_hash` is in the returned `UserRow`. The caller now has access to the password hash. +- A caller that logs `user` logs the hash. A caller that serializes `user` serializes the hash. +- **Fix:** `User` does not include `password_hash`. Only `UserRepository` and `AuthService` (internal) see it. + +### C4 Locality (Core) +- The SQL is in the repo, but the column knowledge (`password_hash`, `deleted_at`) is in the caller. +- A column rename touches the repo AND every caller. Locality is violated. +- **Fix:** Column names are local to the repo. The caller knows only the domain `User`. + +### C5 Reversibility (Core) +- Changing the database (e.g., from SQL to NoSQL, or renaming a column) requires touching every caller. +- The abstraction was supposed to make the change local. It does not. +- **Fix:** The repo's interface (`findByEmail`, `save`) is stable. The implementation changes; the callers do not. + +## What This Example Reveals + +The leaky abstraction is the false promise of encapsulation. The class is named `UserRepository` (suggesting it abstracts persistence), but it returns raw DB rows, exposes lifecycle columns, and leaks secret fields. The abstraction exists in name only. + +The cost: +- Every caller must know the DB schema (C6 violated). +- A schema change touches every caller (C5 violated, C4 violated). +- Secret fields leak to callers (Security P9 violated). +- The lifecycle is the caller's burden (Data P8 violated). + +The fix is always the same: **the abstraction's public type is the domain type, not the implementation type**. `UserRepository.findByEmail()` returns `User | null`, where `User` has `id`, `email`, `name` — and nothing else. `password_hash`, `deleted_at`, `UserRow` are internal. The caller knows nothing about the database. \ No newline at end of file diff --git a/examples/bad/silent-error.md b/examples/bad/silent-error.md new file mode 100644 index 0000000..996644e --- /dev/null +++ b/examples/bad/silent-error.md @@ -0,0 +1,88 @@ +# Bad Example: Silent Error + +> An error-handling pattern that violates Atelier principles. Each violation is cited. + +## The Code + +```typescript +async function getUser(id: string): Promise { + try { + const user = await db.query('SELECT * FROM users WHERE id = $1', [id]); + return user; + } catch (e) { + return null; + } +} + +async function processOrder(orderId: string): Promise { + const order = await getOrder(orderId); + if (!order) { + return; // silently do nothing + } + // ... process +} + +// Usage in a route +router.get('/users/:id', async (req, res) => { + const user = await getUser(req.params.id); + if (!user) { + res.status(404).json({ error: 'Not found' }); + } else { + res.json({ data: user }); + } +}); +``` + +## Violations + +### Errors P2 Fail Loudly (Errors) +- `catch (e) { return null }` swallows the error. The caller cannot distinguish "user not found" from "database down." +- A database outage returns 404s. The operator never knows. Silent failure. +- **Fix:** Catch and re-throw with context, or return a typed error (`Result`). Never `null` for "an error happened." + +### Errors P3 Fail Specifically (Errors) +- `return null` is the least specific response. It could mean: not found, db error, network error, permission error. +- The caller's `if (!user)` cannot distinguish these. The 404 is a lie if the real cause was a 500. +- **Fix:** Return `Result` or throw. The error type/code carries the specificity. + +### Errors P1 Errors are Data (Errors) +- `null` is not data. It is the absence of data. Conflating "error" with "absence" loses information. +- The error (a database failure) was data; it was thrown away and replaced with `null`. +- **Fix:** Errors are values. Return the error value, not a sentinel absence. + +### Errors P4 Preserve Context (Errors) +- The catch block discards `e`. The stack trace, the error message, the cause — all gone. +- The log has no record. The operator cannot debug. +- **Fix:** Log the error with context. Wrap and re-throw: `throw new Error('getUser failed', { cause: e })`. + +### Errors P9 Errors are Logged (Errors) +- The error is not logged. The handling (return null) is the entire response. The log is missing. +- An error that is not logged is an error that cannot be investigated. +- **Fix:** `logger.error({ err: e, userId: id })` before returning/rethrowing. + +### Errors P10 Errors Don't Lie (Errors) +- `return null` claims "no user" when the truth may be "database down." The function lies. +- The 404 response claims "not found" when the truth may be "internal error." The API lies. +- **Fix:** The response status must match the actual condition. 500 for server errors, 404 for not found. + +### Observability P2 Correlation, P3 Context (Observability) +- No `request_id`. No correlation across services. +- No context in the (missing) log. "What was the user doing?" is unanswerable. +- **Fix:** Propagate `request_id`. Log with path, method, user_id. + +### Security P8 Fail Securely (Security) +- The silent failure is fail-open in disguise. If `getUser` fails due to an authz check throwing, the catch returns `null`. +- The caller treats `null` as "not found" and may proceed, or may 404. Either way, the security failure is hidden. +- **Fix:** Distinguish "not found" (404) from "authz error" (403) from "db error" (500). Never collapse them into `null`. + +## What This Example Reveals + +The silent error is the most common and most damaging anti-pattern. It violates Errors P2 (Fail Loudly), P3 (Fail Specifically), P1 (Errors are Data), P4 (Preserve Context), P9 (Errors are Logged), P10 (Errors Don't Lie) — six of ten error principles in one catch block. + +The downstream effects: +- Operators cannot debug (no log, no context). +- Users see wrong errors (404 for a 500). +- Security failures hide (authz error becomes "not found"). +- The system appears healthy when it is not (no metrics, no logs). + +The fix is always the same: **never swallow an error**. Log it, wrap it, rethrow it, or return it as a typed value. Never `return null` for "something went wrong." \ No newline at end of file diff --git a/examples/good/api-endpoint.md b/examples/good/api-endpoint.md new file mode 100644 index 0000000..5c928c5 --- /dev/null +++ b/examples/good/api-endpoint.md @@ -0,0 +1,79 @@ +# Good Example: API Endpoint + +> A REST endpoint that follows Atelier's API principles. Each aspect cites the principle it satisfies. + +## The Endpoint + +```typescript +// POST /v1/orders — create an order +router.post('/v1/orders', auth, validate(CreateOrderSchema), async (req, res) => { + const { userId, items } = req.body; + + const order = await orderService.create({ userId, items }); + + res.status(201).location(`/v1/orders/${order.id}`).json({ + data: order, + }); +}); +``` + +## What Makes It Good + +### Resource Naming (API P2 Clarity, P3 Predictability) +- `/v1/orders` — noun, plural, lowercase, hyphenated. +- Versioned (`/v1`) — P5 Versioning. +- No verb in the URL; the HTTP method is the verb. + +### Method Semantics (API P1 Contract Fidelity, P6 Idempotency) +- POST for creation. 201 on success (not 200). `Location` header for the new resource. +- Idempotency key supported via middleware (omitted for brevity) — P6. + +### Authentication (API P8 Security, Security P1 Zero Trust) +- `auth` middleware runs on every endpoint by default. No opt-in auth. +- The endpoint does not re-implement auth; it relies on the boundary check. + +### Input Validation (API P8, Security P4 Input Validation) +- `validate(CreateOrderSchema)` — schema-based validation at the boundary. +- The schema (zod, joi, etc.) defines types, ranges, required fields. +- Unknown fields rejected (`additionalProperties: false` in the schema). + +### Response Shape (API P2 Clarity) +- `{ data: order }` — wrapped, not a bare object. Allows adding pagination/metadata without breaking. +- The shape is consistent across all endpoints in the API. + +### Error Handling (API P9 Error Transparency, Errors P1 Errors are Data) +- Errors thrown in `orderService.create` are caught by centralized middleware. +- Errors are structured: `{ error: { code, message, request_id } }`. +- 404 → `ORDER_NOT_FOUND`, 409 → `DUPLICATE_ORDER`, 422 → `VALIDATION_FAILED`. + +### Observability (Observability P2 Correlation, P3 Sufficient Context) +- `request_id` propagated via middleware. Every log in the request includes it. +- Significant events logged: "order created", "order creation failed". + +### Economy (Core C8, Performance P4 Resource Bounds) +- The order creation is bounded in time (the service has a timeout). +- No unbounded query; no loading all products into memory. + +## The Schema (for completeness) + +```typescript +const CreateOrderSchema = z.object({ + userId: z.string().uuid(), + items: z.array(z.object({ + productId: z.string().uuid(), + quantity: z.number().int().positive().max(100), + })).min(1).max(50), +}).strict(); // additionalProperties: false +``` + +- `userId` is a UUID (Data P7 Type Fidelity). +- `quantity` is bounded (Performance P4). +- `items` is bounded (1–50) (Performance P4, Security P10 Surface Minimization). +- `.strict()` rejects unknown fields (Security P4). + +## What This Example Does NOT Do (And Why That's Good) + +- Does not return 200 on error — the status code is the first signal (API P9). +- Does not log the request body — may contain PII (Observability P6, Security P9). +- Does not construct SQL by string interpolation — uses a service layer (Security P5 Output Safety). +- Does not skip auth for "internal" callers — Zero Trust (Security P1). \ No newline at end of file diff --git a/examples/good/db-schema.md b/examples/good/db-schema.md new file mode 100644 index 0000000..70c5cdd --- /dev/null +++ b/examples/good/db-schema.md @@ -0,0 +1,88 @@ +# Good Example: Database Schema + +> A SQL schema that follows Atelier's Data principles. Each aspect cites the principle it satisfies. + +## The Schema + +```sql +-- Users table +CREATE TABLE users ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + email VARCHAR(255) NOT NULL, + name VARCHAR(100) NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + deleted_at TIMESTAMPTZ, + + CONSTRAINT users_email_unique UNIQUE (email), + CONSTRAINT users_email_format CHECK (email ~ '^[^@]+@[^@]+\.[^@]+$') +); + +CREATE INDEX users_email_idx ON users (email) WHERE deleted_at IS NULL; +CREATE INDEX users_created_at_idx ON users (created_at DESC); + +-- Orders table +CREATE TABLE orders ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + user_id UUID NOT NULL REFERENCES users(id) ON DELETE RESTRICT, + status VARCHAR(20) NOT NULL DEFAULT 'pending', + total_cents INTEGER NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + CONSTRAINT orders_status_valid CHECK (status IN ('pending', 'paid', 'shipped', 'cancelled')), + CONSTRAINT orders_total_positive CHECK (total_cents >= 0) +); + +CREATE INDEX orders_user_id_idx ON orders (user_id); +CREATE INDEX orders_status_created_idx ON orders (status, created_at DESC); +``` + +## What Makes It Good + +### Truth (Data P1) +- The schema reflects the domain: `users` have `email`, `name`, lifecycle timestamps. `orders` have `status`, `total`. +- No column named after a feature (`is_active_for_X`). No application state in the schema. + +### Invariants in the Schema (Data P3) +- `NOT NULL` where required: `email`, `name`, `user_id`, `status`, `total_cents`. +- `UNIQUE (email)` — emails are unique. Enforced in the DB, defended in the app. +- `CHECK (status IN (...))` — status is a finite domain. Enforced in the DB. +- `CHECK (total_cents >= 0)` — totals are non-negative. Enforced in the DB. +- `REFERENCES users(id) ON DELETE RESTRICT` — you cannot delete a user with orders. Referential integrity (P9). + +### Type Fidelity (Data P7) +- `id` is `UUID`, not `VARCHAR(36)`. The type matches the domain. +- `created_at` is `TIMESTAMPTZ`, not `VARCHAR` or `INTEGER`. Timezone-aware. +- `total_cents` is `INTEGER`, not `FLOAT`. Money in cents avoids floating point (P1 Truth). +- `status` is `VARCHAR(20)` with a CHECK, not a free `TEXT`. Bounded. + +### Naming Consistency (Data P6) +- snake_case: `users`, `orders`, `user_id`, `created_at`. +- Foreign key: `user_id` (singular table + `_id`), not `uid` or `user`. +- Timestamps: `created_at`, `updated_at`, `deleted_at` — consistent suffix `_at`. + +### Lifecycle Awareness (Data P8) +- `deleted_at` for soft delete. Lifecycle is first-class. +- The unique index on email is partial: `WHERE deleted_at IS NULL` — allows re-registration after soft delete. +- Every query must filter `deleted_at IS NULL` (a discipline, not a schema property). + +### Indexing with Intent (Data P5, P10) +- `users_email_idx` — queries by email (login, lookup). Partial (excludes soft-deleted). +- `users_created_at_idx` — list users by recency. DESC matches the typical query. +- `orders_user_id_idx` — list a user's orders. FK index (join performance). +- `orders_status_created_idx` — composite for "open orders by recency" (`WHERE status = 'pending' ORDER BY created_at DESC`). +- No index on every column. Each index serves a query. + +### Migration Safety (Data P4) +- This schema is created via a migration with an `up` and a `down`. +- The `down` drops the tables in reverse order (orders, then users) to respect FKs. +- Adding a column later uses expand-contract (nullable first, then constrained). + +## What This Example Does NOT Do (And Why That's Good) + +- Does not use `FLOAT` for money — floating point errors (P7, P1). +- Does not use `VARCHAR` for the UUID — wrong type (P7). +- Does not omit the FK on `orders.user_id` — unenforced relationship (P9). +- Does not index every column — write amplification (P10, C8). +- Does not use `is_deleted BOOLEAN` without a timestamp — loses the deletion time (P8). +- Does not allow `status` to be free text — would lose the finite domain (P3). \ No newline at end of file diff --git a/examples/good/error-handler.md b/examples/good/error-handler.md new file mode 100644 index 0000000..bcaf524 --- /dev/null +++ b/examples/good/error-handler.md @@ -0,0 +1,139 @@ +# Good Example: Error Handler + +> An error handler that follows Atelier's Errors principles. Each aspect cites the principle it satisfies. + +## The Handler + +```typescript +// Centralized error middleware +import { v4 as uuidv4 } from 'uuid'; + +type AppError = { + code: string; + message: string; + statusCode: number; + details?: Record; + cause?: unknown; +}; + +function errorHandler(err: unknown, req: Request, res: Response, next: NextFunction) { + const requestId = req.headers['x-request-id'] as string || uuidv4(); + + const appError = normalizeError(err, requestId); + + // Log with full context (server-side only) + logger.error({ + request_id: requestId, + code: appError.code, + message: appError.message, + details: appError.details, + path: req.path, + method: req.method, + user_id: req.user?.id, + stack: err instanceof Error ? err.stack : undefined, + }); + + // Respond with safe, structured error + res.status(appError.statusCode).json({ + error: { + code: appError.code, + message: appError.message, + request_id: requestId, + ...(appError.details ? { details: appError.details } : {}), + }, + }); +} + +function normalizeError(err: unknown, requestId: string): AppError { + // Known error types — preserve their code and status + if (err instanceof ValidationError) { + return { + code: err.code, + message: err.message, + statusCode: 422, + details: err.details, + }; + } + if (err instanceof NotFoundError) { + return { code: err.code, message: err.message, statusCode: 404 }; + } + if (err instanceof AuthError) { + return { code: 'AUTH_REQUIRED', message: 'Authentication required', statusCode: 401 }; + } + + // Unknown error — fail securely, do not leak internals + return { + code: 'INTERNAL_ERROR', + message: 'An internal error occurred', + statusCode: 500, + cause: err, + }; +} +``` + +## What Makes It Good + +### Errors are Data (Errors P1) +- Errors are normalized into a structured `AppError` type. They are values, not raw exceptions. +- The `code` is a stable, machine-consumable string (`VALIDATION_FAILED`, not a free message). +- The handler treats every error as data to be transformed, not a flow to be caught. + +### Fail Loudly (Errors P2) +- No `catch (e) {}`. Every error reaches the handler. +- No silent swallowing. The error is logged and responded to. +- The middleware is centralized — every route uses it. No route can "forget" to handle errors. + +### Fail Specifically (Errors P3) +- `ValidationError` → 422 with `code: err.code` (specific, e.g., `INVALID_EMAIL`). +- `NotFoundError` → 404 with `code: err.code` (e.g., `USER_NOT_FOUND`). +- Unknown → 500 with `INTERNAL_ERROR` (generic only when truly unknown). +- Never "Something went wrong" — always a specific code. + +### Preserve Context (Errors P4) +- Server-side log includes: `request_id`, `code`, `message`, `details`, `path`, `method`, `user_id`, `stack`. +- Client-side response includes: `code`, `message`, `request_id`, `details`. +- The `request_id` ties the client response to the server log. Debuggable. + +### Recoverable When Possible (Errors P5) +- 422 (validation) — the client can fix and retry. +- 401 (auth) — the client can re-auth and retry. +- 404 (not found) — the client can correct the ID. +- Only 500 is "the server has a bug" — not recoverable by the client. + +### Unrecoverable Means Stop (Errors P6) +- A 500 does not limp on. It logs and returns. +- The server does not try to "recover" from an unknown error by guessing. It fails fast. + +### Errors are Boundaries (Errors P7) +- The middleware is the boundary. Internal error types (`ValidationError`, `NotFoundError`) are translated to HTTP responses here. +- Internal code throws domain errors; the boundary translates to HTTP. No internal error type leaks to the client. + +### User-Facing Errors are UX (Errors P8) +- `message` is for the developer (log + response). The client UI renders a user-facing message based on `code`. +- `INTERNAL_ERROR` → "Something went wrong. We're on it." (user-facing), not the stack. + +### Errors are Logged (Errors P9) +- Every error is logged at ERROR level with full context. +- The handling is the recovery (return a response); the log is the memory (investigate later). + +### Errors Don't Lie (Errors P10) +- The status code matches the error type. 422 for validation, not 200. +- `INTERNAL_ERROR` is returned only for unknown errors. Known errors get their specific code. +- Never claim success on failure. Never claim failure on success. + +### Security (Security P5 Output Safety, P9 Secret Hygiene) +- Unknown errors return `INTERNAL_ERROR` with no internal details. No stack trace to the client. +- The stack is logged server-side, never sent to the client. +- The error message does not echo the input (which may contain a token). + +### Observability (Observability P2 Correlation, P3 Context) +- `request_id` on every error. Correlatable across services. +- Sufficient context in the log: path, method, user_id. "What was the user doing?" is answerable. + +## What This Example Does NOT Do (And Why That's Good) + +- Does not `catch (e) { return null }` — silent failure (P2). +- Does not return 200 with `{ error: ... }` — the status code lies (API P9). +- Does not send the stack trace to the client — information leak (Security P5). +- Does not log the request body — may contain secrets (Security P9, Observability P6). +- Does not use a generic `Error("error")` — not specific (P3). \ No newline at end of file diff --git a/examples/good/react-component.md b/examples/good/react-component.md new file mode 100644 index 0000000..34f613b --- /dev/null +++ b/examples/good/react-component.md @@ -0,0 +1,108 @@ +# Good Example: React Component + +> A UI component that follows Atelier's UI/UX principles. Each aspect cites the principle it satisfies. + +## The Component + +```tsx +import { useId, useState } from 'react'; +import { Button } from './Button'; +import { Spinner } from './Spinner'; + +type DeleteButtonProps = { + /** The resource name to display in the confirmation */ + resourceName: string; + /** Called when the user confirms deletion */ + onDelete: () => Promise; +}; + +export function DeleteButton({ resourceName, onDelete }: DeleteButtonProps) { + const [isConfirming, setIsConfirming] = useState(false); + const [isDeleting, setIsDeleting] = useState(false); + const confirmId = useId(); + + const handleConfirm = async () => { + setIsDeleting(true); + try { + await onDelete(); + } finally { + setIsDeleting(false); + setIsConfirming(false); + } + }; + + if (isConfirming) { + return ( + + Delete {resourceName}? This cannot be undone. + + + + ); + } + + return ( + + ); +} +``` + +## What Makes It Good + +### Single Responsibility (components.md §1) +- The component does one thing: confirm and trigger a deletion. No "And" in the name. +- The `onDelete` callback is the single output. The component owns the confirmation UI, not the deletion logic. + +### Explicit Boundaries (components.md §3) +- Props are typed (`DeleteButtonProps`). Required props are required. +- `onDelete` returns a `Promise` — the caller knows it's async. +- The component never reads global state. It receives `resourceName` and `onDelete`. + +### Predictable State (components.md §4) +- `isConfirming` and `isDeleting` are owned by the component (only it cares). +- State is not duplicated. The parent does not know about confirmation. +- The component transitions: idle → confirming → deleting → idle. + +### Render Purity (components.md §5) +- Given the same props and state, the component renders the same output. +- Side effects (`onDelete`) are in the event handler, not in render. +- `useId()` is deterministic per component instance (React guarantee). + +### Accessible by Default (components.md §6, uiux P2) +- The confirmation group has `role="group"` and `aria-labelledby`. +- The Spinner has a `label` (screen reader announces "Deleting"). +- Buttons have text labels (not icon-only). +- Focus order is logical (confirm → cancel). +- Keyboard-reachable (buttons are natively focusable). + +### Forgiveness (UI/UX P5, P10 Reversibility) +- Destructive action requires confirmation (P5). +- "This cannot be undone" names the consequence (P3 Clarity). +- "Cancel" is offered and is not disabled during deletion (the user can cancel the *next* action). +- The state is reversible: `isConfirming` can be set back to `false` (P10). + +### Style via Tokens (components.md §7) +- `variant="danger"` and `variant="ghost"` reference design tokens, not raw colors. +- No `style={{ color: 'red' }}` — the token system owns the visual. + +### Feedback (UI/UX P4) +- The button shows a Spinner while deleting (P4, P6 Performance perception). +- The button is disabled while deleting (prevents double-click). +- The label changes: "Yes, delete" → Spinner (state is communicated). + +### Clarity (UI/UX P3) +- "Delete {resourceName}? This cannot be undone." — specific, names the resource and the consequence. +- No "Are you sure?" — vague. No "Submit" — wrong verb. + +## What This Example Does NOT Do (And Why That's Good) + +- Does not use a `window.confirm()` dialog — not accessible, not styled, not composable. +- Does not render a modal — the inline confirmation is lighter and less disruptive (P9 Simplicity). +- Does not auto-delete on click — forgiveness (P5). +- Does not hardcode "Project" — the resource name is a prop (composability, components.md §3). \ No newline at end of file