Commit graph

3 commits

Author SHA1 Message Date
95bd8fab87 refactor: move ErrorHandlerService/HttpError out of express-tools
ErrorHandlerService has zero dependency on Express — it's a plain
"map an error to {status, body}" service that works identically
behind any HTTP framework. It had no business living in a package
named express-tools.

Extracted HttpError, ErrorHandlerService, and ErrorHandlingResult
into a new packages/error-tools package (same tsc-build-to-dist
pattern as shared/express-tools). express-tools now only keeps the
actual Express-specific layer: ExpressServer, wrapAsyncHandler, and
createErrorMiddleware (which adapts ErrorHandlerService, imported
from error-tools, onto Express).

- packages/error-tools: new package, depends on shared + zod
- packages/express-tools: drops zod dependency, adds error-tools
  dependency for error-middleware.ts's type import
- apps/api: adds error-tools dependency; app.ts, auth.service.ts,
  require-auth.ts now import HttpError/errorHandlerService from
  error-tools instead of express-tools
- apps/api/Dockerfile: adds COPY for packages/error-tools in the
  runtime stage
- specs/error-handling.md, specs/backend-architecture.md, README.md
  updated to reflect the new package split

Verified: pnpm lint, pnpm build (all packages, correct dependency
order), pnpm test (9/9 Mocha), pnpm test:bdd (5/5 Cucumber), full
Docker rebuild + compose up (no crash-loop), curl + browser checks
of /health, unknown-route 404, signup (201), duplicate-email 409
(code 4001 EMAIL_ALREADY_IN_USE) — all going through the moved
ErrorHandlerService/HttpError correctly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-16 18:21:09 +02:00
1cedb25d74 Address review: no .d.ts, express-tools package, faker fixtures, numeric codes, real i18n lib
Five explicit review points, addressed on this same PR branch (not a new
PR) per updated preference.

## No .d.ts files in the codebase

- apps/web: vite-env.d.ts removed — its /// <reference types="vite/client" />
  is replaced by "types": ["vite/client"] in tsconfig.app.json, same effect.
- apps/api: src/types/express.d.ts renamed to express-request.augment.ts —
  `declare global` module augmentation works identically in a plain .ts
  file as long as it has a top-level import (making it a module); the
  .d.ts extension wasn't doing anything for us here.

## packages/express-tools — separate package for Express tooling

Moved HttpError and ErrorHandlerService out of apps/api into a new
workspace package, plus a new createErrorMiddleware() factory (the actual
Express 4-arg error-handling middleware, previously inlined in app.ts).
apps/api now just consumes @batch-cooking/express-tools. Has a real build
(tsc -> dist/, same pattern as packages/shared) — required for the same
reason shared needed one: apps/api's Docker image runs plain `node
dist/server.js`, no tsx. apps/api/Dockerfile updated to COPY the new
package's dist alongside shared's.

## faker.js for test fixtures

apps/api/test/auth.test.ts: replaced the hardcoded "Nicolas
Lefevre"/nicolas@example.com fixture (looked like real user data) with
@faker-js/faker, generated fresh per test via buildSignupPayload().
features/step-definitions/auth.steps.ts: fakerized the filler
firstName/lastName/password used for background state the scenarios
don't actually read.

Deliberately did NOT fakerize the literal example values inside
auth.feature itself (alice@example.com etc.) — those are the readable,
illustrative Gherkin examples that are the whole point of BDD scenarios,
not real PII, and randomizing them would make the scenarios harder to
read for no real gain. Flagged this reasoning in the README in case that
call should go the other way.

Caught a real bug while wiring this up: faker.internet.email() sometimes
capitalizes parts of the address, but signupSchema/loginSchema normalize
emails to lowercase — the test fixture needs to match what's actually
stored, so buildSignupPayload() lowercases the generated email too.
Found by actually running the suite repeatedly, not just once.

## ErrorCode: numeric enum, zero hardcoded values

packages/shared/src/errors/error-codes.ts: ErrorCode is now a numeric
enum (4000 VALIDATION_ERROR, 4001 EMAIL_ALREADY_IN_USE, 4010
INVALID_CREDENTIALS, 4011 NOT_AUTHENTICATED, 4040 NOT_FOUND, 5000
INTERNAL_ERROR — grouped by family like HTTP status codes).

Audited and fixed every place that hardcoded a raw code value instead of
referencing the enum: ApiClient's fallback (`"INTERNAL_ERROR" as
ErrorCode` — would no longer even type-check once the enum went numeric,
which is exactly the point), and the Cypress mock bodies (now import
ErrorCode from @batch-cooking/shared instead of typing the string).

Cucumber's "the response error code should be {string}" step still takes
the *name* in the .feature file (readable: "EMAIL_ALREADY_IN_USE") and
resolves it to the real numeric value via ErrorCode[name] — TypeScript's
reverse enum mapping — before comparing, so the Gherkin stays readable
without the step hardcoding a number either.

## Real i18n library (i18next), not a hand-rolled label map

apps/web: added i18next + react-i18next. New locales/fr/translation.json
holds every user-facing string — not just error labels (errors.*), but
the login/signup/home pages' labels, buttons and headings too
(auth.login.*, auth.signup.*, home.*) — via useTranslation()/t() in each
page. ErrorMessageService no longer owns its own label map; it converts
the numeric ErrorCode to its enum member name and delegates the actual
lookup to i18next (errors.<MEMBER_NAME>). Adding a language is now
"add a locale file", not a code change anywhere.

## specs/ and README updated

specs/error-handling.md and specs/frontend-architecture.md rewritten for
the new package, numeric codes, and i18next. New "i18n" and "no .d.ts"
sections. README covers the same, plus a note on the faker.js scope
decision (feature-file literals excluded, on purpose).

## Verification

Full lint/mocha (x3 runs)/cucumber/build green. Re-verified
express-tools' extraction against a real risk (not just tsc passing):
ran `node dist/server.js` standalone (mirrors the Docker runtime, no
tsx) and hit /health, a 404 (confirmed numeric code 4040 over the wire),
and a real signup + duplicate-email 409 (confirmed numeric 4001). Then
re-verified the full pipeline in a real browser against native dev
servers: signup, EMAIL_ALREADY_IN_USE -> i18next -> "Cet email est déjà
utilisé" end-to-end, home page i18next interpolation
({{firstName}}/{{lastName}}) rendering correctly.
2026-08-16 16:22:29 +02:00
kyuno053
bc582b75d6
Add Docker packaging for local functional review (api + web) (#5)
apps/api/Dockerfile: multi-stage build on node:22-slim (not alpine —
avoids musl-vs-glibc native binding surprises for argon2/Prisma's
engine binaries; same base image family for build and runtime stages
keeps "native" binaries compatible across stages). Runtime stage
copies the monorepo structure as-is rather than flattening to a single
package, so pnpm's symlinked node_modules stay valid. Container runs
`prisma migrate deploy` on startup before starting the server, so the
review environment's schema is always in sync automatically.

Installs openssl explicitly in the base image: without it, Prisma
can't detect the right engine binary and silently defaults to a guess
that may not match what's actually on the image — caught by checking
the build log, not just a successful build.

apps/web/Dockerfile: builds with Vite, serves the static output via
nginx (not a Node static server) — avoids the devDependency problem of
needing `vite preview` in a --prod-deployed image, and is the more
standard way to serve a built SPA. nginx.conf has an SPA fallback
(try_files ... /index.html) ready for when client-side routing lands.

docker-compose.yml: adds `api` and `web` services alongside the
existing `postgres`. api's DATABASE_URL targets the `postgres` service
name over the compose network (not localhost/POSTGRES_PORT, which is
only the host-side mapping). Both new services require JWT_SECRET/
ports via env vars with no defaults, consistent with the project's
existing no-hardcoded-credentials rule.

.dockerignore added — without it, the Windows-built node_modules
(with Windows-specific native binaries) would get copied into the
Linux build context.

Verified: full build (api + web images), `docker compose up -d`
brought up all three containers, curled /health and the web root,
ran a real signup through the containerized stack end-to-end.
2026-08-16 14:13:51 +02:00