Repository navigation
feat(db): verify schema at startup and expose /ready - #553
titankrypt-gif wants to merge 1 commit into
Conversation
|
@titankrypt-gif Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
left a comment
There was a problem hiding this comment.
The schema verification itself is good work and #352 is yours — but two things need to change, and one of them now stops the app booting.
1. The duplicate /ready breaks boot
#544 (issue #334, /live and /ready probes) merged a few minutes ago, and it registers /ready in src/routes/health.ts. This PR registers it again in src/api/rest.ts. Fastify refuses duplicate routes, so the app no longer starts. I merged your branch onto current main and ran it rather than guessing:
FastifyError: Method 'GET' already declared for route '/ready'
tests/openapi.test.ts fails to load at all as a result, because it boots the real app.
/ready belongs to #334, which was assigned to CollinsKRO and landed first, so that part comes out of this PR. That is not a judgement about which implementation is better — it's that the issue was theirs.
2. EXPECTED_SCHEMA has nothing stopping it drifting
This is the more interesting problem, and it's the one I'd most like you to solve.
EXPECTED_SCHEMA is a hand-written list of ten tables and their columns, taken from prisma/schema.prisma's @@map/@map names. Your comment explains why it's explicit rather than a DMMF walk — it has to work in the deployed container and stay cheap — and I agree with that reasoning.
But nothing ties the list to the Prisma schema. So the next migration that adds a column leaves this check passing while the thing it exists to verify has drifted, and a migration that renames one makes it fail on a database that is actually correct. A drift guard that drifts is worse than none, because it is trusted.
This repo already solves exactly this shape of problem twice: src/__tests__/graphqlSchemaDrift.test.ts byte-compares schema.graphql against printSchema() output, and tests/openapi.test.ts enumerates registered routes and fails on any missing from openapi.yaml. Both keep a hand-maintained artifact honest with a test rather than with discipline.
What I'd like is the same pattern here: a test that parses prisma/schema.prisma, derives the mapped table and column names, and asserts EXPECTED_SCHEMA matches exactly — failing in both directions, with a message naming what to add or remove. Then the runtime check stays the cheap two information_schema queries you wrote, and the list cannot rot.
What I'd suggest instead of a second route
Keep verifyDatabaseSchema and its startup call, drop the route, and surface the result through #544's readiness path in src/health.ts. A "the database schema does not match the code" signal is genuinely a readiness concern — the process is up but must not take traffic — so it belongs as a check inside the existing /ready rather than as a competing endpoint. That also gets you the caching and the 503 shape for free.
What's already right
- Letting query failures propagate so an outage is distinguishable from a mismatch. That distinction is the whole value of the check and it would have been easy to swallow.
- Caching the successful result, since the schema cannot change under a running process.
- Parameterising the table list into
= ANY($1::text[])rather than interpolating it. - Not querying columns for tables already known to be missing.
- Five tests covering missing table, missing column, the startup throw, and the connection-failure case separately.
#352 stays yours and stays assigned to you. Drop the route, add the drift guard, and I'll re-review quickly — I'm working through the queue continuously today.
One unrelated note: tests/aggregator.property.test.ts also failed on my merge run, on a random-input case. I don't think it's yours — it's a property test and it passes on main — but if you see it, that's why.
|
Closing as superseded. #352 was already resolved on main by #537 (merged 2026-10-22), which added src/db/schemaVerify.ts, the startup assertion, the /ready probe, tests and docs/database-schema.md. This branch now conflicts with that implementation and would duplicate it, so it should not be merged. |
Summary
Adds a cheap startup schema check, a public
/readyprobe that reports schema readiness, tests for the mismatch paths, and documentation of why the repository has no Prisma migrations yet.Problem
prisma/has onlyschema.prismaand no migrations directory. A deployed process could start cleanly against a database missing a table or column and only fail later, at request time, with an opaque query error. There was also no readiness signal that distinguishes "the process is up" from "this instance can serve the schema it was built for".Solution
src/db/schemaVerify.ts:EXPECTED_SCHEMAlists the physical tables/columns used by the code (Prisma@@map/@mapnames).verifyDatabaseSchema(pool)runs twoinformation_schemaqueries and reports missing tables and columns.assertSchemaValid(pool)throwsDatabaseSchemaMismatchErrornaming every missing entity./readydoes not re-query on every probe.src/index.tscallsassertSchemaValid(pgPool)after the database connection and before loading pairs or listening. A mismatch exits through the existing fatal-startup path.GET /readyreturns200withready: truewhen the cached check passed, and503withmissingTables/missingColumnsotherwise. Database outages return503withdatabase_unavailablerather than being misreported as schema drift./readyis public and included in the rate-limit allow-list, like/status.docs/database-schema.mdrecords the currentdb pushstrategy and rationale, the guard,/ready, and the concrete path to Prisma Migrate.openapi.yaml/openapi.jsondocument the new endpoint./ready200/503, caching, and the OpenAPI coverage guard.Validation
npx tsc --noEmit- passnpx vitest run src/__tests__/schemaVerification.test.ts src/__tests__/ready.test.ts src/__tests__/status.test.ts tests/openapi.test.ts- 17 passed, 0 failednpm test- 831 passed, 1 skipped, 1 failed:graphqlSchemaDrift.test.tscompares the LF-generated SDL against the CRLF checkout on this Windows workspace.npm run graphql:schema:genproduces no content diff andgit checkout -- schema.graphqlleaves the tree clean, so this is a local line-ending artifact rather than a schema change.Risk
prisma db push, so it verifies the result of that push rather than replacing it. The docs call out that adopting Prisma Migrate (baseline, resolve, deploy pipeline) is the follow-up that should remove the boot-time push.EXPECTED_SCHEMAis intentionally explicit. A future model change must add its table/columns there; the startup check will not silently ignore a newly expected field.Issue
Closes #352