fix(server): Postgres integration tests + unique-violation detection bug#4
Merged
Conversation
Add a real-Postgres integration tier for the Drizzle stores and fix a bug it immediately surfaced. Bug: isUniqueViolation only checked the top-level error's `code === "23505"`, but Drizzle wraps the driver error in `.cause` and Bun's SQL driver exposes the SQLSTATE as `errno` (with `code` set to the generic ERR_POSTGRES_SERVER_ERROR). So a duplicate username threw a raw error that surfaced as 503 AUTH_UNAVAILABLE instead of a clean 409 USERNAME_TAKEN. Now walk the cause chain and check code, errno, and the message at each level. The fake-double unit tests could not catch this because they never hit a real driver. Tests / CI: - apps/server/src/db/integration.test.ts exercises DrizzleAuthStore, DrizzleFriendStore, and DrizzlePersistenceStore against real Postgres: column mappings, the username unique constraint, token-version revocation, canonical friendships listed both directions, the no-self-friend CHECK, and room/session persistence. Opt-in via RUN_DB_TESTS=1 so the default `bun test` stays database-free. - ci.yml: add a postgres:16 service, RUN_DB_TESTS=1, and a db:migrate step before the test run (which also validates migrations, including 0008). - Add a test:integration script. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up #1 from the review: a real-Postgres integration tier for the Drizzle stores (review finding 7.5 — they were previously only exercised against fake chainable doubles). It immediately paid for itself by catching a production bug.
Bug found and fixed
isUniqueViolationonly checked the top-level error'scode === "23505". But Drizzle wraps the driver error in.cause, and Bun's SQL driver puts the SQLSTATE inerrno(withcodeset to the genericERR_POSTGRES_SERVER_ERROR). So a duplicate username threw a raw error that surfaced as 503AUTH_UNAVAILABLEinstead of a clean 409USERNAME_TAKEN. The fix walks thecausechain and checkscode,errno, and the message at each level. The fake-double unit tests couldn't catch this — only a real driver does.Integration tests
apps/server/src/db/integration.test.tsexercises all three stores against real Postgres:DrizzleAuthStore: column mapping,username_keyunique constraint →UsernameTakenError, token-version revocation round-trip.DrizzleFriendStore: canonical friendship storage,countFriends, bidirectionallistFriends, and the no-self-friendCHECKconstraint.DrizzlePersistenceStore: room seeding, visibility/owner filtering, and last-room session save/get/clear.Opt-in via
RUN_DB_TESTS=1so the defaultbun teststays database-free (skips with a placeholder when the var is unset).CI
postgres:16service +RUN_DB_TESTS=1+ adb:migratestep before tests (which also validates the migration set, including the new0008).test:integrationscript for local runs (bun run db:upfirst).Test plan
bun run lint✅ ·bun run typecheck✅bun test(no DB) ✅ 250 pass, 1 skip ·bun run coverage:check✅ 95.24%RUN_DB_TESTS=1 bun run test:coverage(real PG) ✅ 254 pass ·coverage:check✅ 95.68%Not in this PR (the other two follow-ups)
HttpOnly cookie auth (1.8) and the
Avatar.ts/createApp.tsrefactors (8.1/8.6/5.3) both need a running-app/browser check — cross-origin cookie + CORS-credentials behavior, and pixel-level rendering output respectively (the two "hair switches" turned out to be different renderers sharing a style enum, not a mechanical dedup). I'd rather land those alongside a/verifypass than ship them blind.🤖 Generated with Claude Code