From 9ea8d3371b2335ea3cc82c7eeab086097cc8c942 Mon Sep 17 00:00:00 2001 From: Brandon Estrella Date: Mon, 10 Aug 2026 18:19:58 -0700 Subject: [PATCH 1/2] fix(docker): create the organizations table core's redirect depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every redirect on a self-hosted install failed with `relation "organizations" does not exist` (42P01), taking the whole deployment down. Three separate defects combined to produce it: 1. The redirect lookup LEFT JOINs `organizations` to read `settings.appConfig` (the last link in the ios/android/web URL fallback chain) and joins on `links.organization_id` — but `initializeDatabase()` created neither the table nor the column. Core depended on a table it never shipped. 2. There was no `/health` route. `GET /health` fell through to the catch-all redirect `/:shortCode` and was answered as a short-code lookup, so the documented health check surfaced the same 500 and the container's HEALTHCHECK could never pass. This is why the error looked like it came from the health endpoint. 3. The Dockerfile copied a `migrations/` directory that does not exist in the repo, so `docker build` from source failed outright at that layer. Fixes: - Create a minimal `organizations` table (id, name, settings, suspended_at) and add `links.organization_id`, both idempotent. CREATE TABLE IF NOT EXISTS is a no-op against deployments that already ship a fuller organizations table, so a richer schema is preserved untouched. - Add `/health` (liveness, no DB access) and `/health/ready` (503 when the database is unreachable; Redis reported as degraded, not unready). Static paths beat the parametric redirect route in Fastify's router, so they cannot be shadowed by a short link. - Drop the bogus `COPY migrations` layer and point HEALTHCHECK at `/health/ready`, with an error handler so a dead server exits non-zero. Verified against PostgreSQL 15: reproduced 42P01 on main, then confirmed the fix on a fresh install, on an upgrade of an already-broken database, and against a Cloud-style pre-existing organizations table (columns preserved, suspended_at correctly absent). Fixes #35 --- DOCKER.md | 19 +++++++- Dockerfile | 5 ++- README.md | 7 +++ docker-compose.yml | 14 +++--- src/index.ts | 4 +- src/lib/database.ts | 42 +++++++++++++++++ src/routes/health.test.ts | 94 +++++++++++++++++++++++++++++++++++++++ src/routes/health.ts | 59 ++++++++++++++++++++++++ src/routes/index.ts | 1 + 9 files changed, 234 insertions(+), 11 deletions(-) create mode 100644 src/routes/health.test.ts create mode 100644 src/routes/health.ts diff --git a/DOCKER.md b/DOCKER.md index bf03d2c..19d8b26 100644 --- a/DOCKER.md +++ b/DOCKER.md @@ -149,7 +149,24 @@ docker run --rm -v linkforty_postgres_data:/data -v $(pwd):/backup alpine \ ### Health Checks -The LinkForty container includes a built-in health check: +The server exposes two endpoints: + +| Endpoint | Checks | Use for | +|-----------------|-------------------------------------|--------------------------------------------| +| `/health` | Process is up. Never touches the DB | Liveness probes — a DB blip won't restart you | +| `/health/ready` | Database reachable (+ Redis status) | Readiness probes, load balancer draining | + +`/health/ready` returns `503` when the database is unreachable: + +```bash +curl -s localhost:3000/health/ready +# {"status":"ok","checks":{"database":"ok","redis":"ok"}} +``` + +Redis is an optional cache with database fallback, so a Redis failure is reported +in `checks` but does not make the instance unready. + +The container's built-in `HEALTHCHECK` targets `/health/ready`: ```bash # Check container health diff --git a/Dockerfile b/Dockerfile index 5556e2d..2eccc48 100644 --- a/Dockerfile +++ b/Dockerfile @@ -35,8 +35,9 @@ RUN npm ci --only=production && \ npm cache clean --force # Copy built files from builder +# NOTE: there is no migrations/ directory — the schema is created by +# initializeDatabase() in dist/lib/database.js, which dist/scripts/migrate.js runs. COPY --from=builder /app/dist ./dist -COPY --from=builder /app/migrations ./migrations # Copy example server file COPY examples/basic-server.ts ./ @@ -55,7 +56,7 @@ EXPOSE 3000 # Health check HEALTHCHECK --interval=30s --timeout=10s --start-period=40s --retries=3 \ - CMD node -e "require('http').get('http://localhost:3000/health', (r) => {process.exit(r.statusCode === 200 ? 0 : 1)})" + CMD node -e "require('http').get('http://localhost:3000/health/ready', (r) => {process.exit(r.statusCode === 200 ? 0 : 1)}).on('error', () => process.exit(1))" # Use dumb-init to handle signals properly ENTRYPOINT ["dumb-init", "--"] diff --git a/README.md b/README.md index deb2149..6a4f7bd 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,13 @@ GET /api/sdk/v1/resolve/:shortCode # Resolve link to deep link data (no redi GET /api/sdk/v1/health # Health check ``` +### Health + +```bash +GET /health # Liveness — process is up (no DB access) +GET /health/ready # Readiness — 503 if the database is unreachable +``` + ### Debug & Testing ```bash diff --git a/docker-compose.yml b/docker-compose.yml index f3af36b..87eea99 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -71,13 +71,13 @@ services: - "${LINKFORTY_PORT:-3000}:3000" restart: unless-stopped - # Health check (optional, uncomment if needed) - # healthcheck: - # test: ["CMD", "wget", "--quiet", "--tries=1", "--spider", "http://localhost:3000/health"] - # interval: 30s - # timeout: 10s - # retries: 3 - # start_period: 40s + # /health is liveness (process up); /health/ready also checks the database. + healthcheck: + test: ["CMD", "wget", "--quiet", "--tries=1", "--spider", "http://localhost:3000/health/ready"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 40s volumes: postgres_data: diff --git a/src/index.ts b/src/index.ts index f656902..5cee4a5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ import { webhookRoutes } from './routes/webhooks.js'; import { templateRoutes } from './routes/templates.js'; import { qrRoutes } from './routes/qr.js'; import { wellKnownRoutes } from './routes/well-known.js'; +import { healthRoutes } from './routes/health.js'; /** * Configuration options for creating a LinkForty server instance. @@ -58,6 +59,7 @@ export async function createServer(options: ServerOptions = {}) { await initializeDatabase(options.database); // Routes + await fastify.register(healthRoutes); await fastify.register(wellKnownRoutes); await fastify.register(redirectRoutes); await fastify.register(linkRoutes); @@ -78,4 +80,4 @@ export * from './lib/fingerprint.js'; export * from './lib/webhook.js'; export * from './lib/event-emitter.js'; export * from './types/index.js'; -export { redirectRoutes, linkRoutes, analyticsRoutes, sdkRoutes, webhookRoutes, templateRoutes, qrRoutes, previewRoutes, debugRoutes, wellKnownRoutes } from './routes/index.js'; +export { redirectRoutes, linkRoutes, analyticsRoutes, sdkRoutes, webhookRoutes, templateRoutes, qrRoutes, previewRoutes, debugRoutes, wellKnownRoutes, healthRoutes } from './routes/index.js'; diff --git a/src/lib/database.ts b/src/lib/database.ts index d4c0848..c35c27d 100644 --- a/src/lib/database.ts +++ b/src/lib/database.ts @@ -51,6 +51,29 @@ export async function initializeDatabase(options: DatabaseOptions = {}) { const client = await connectWithRetry(); try { + // Organizations table (must be created before links, which references it). + // + // The redirect path LEFT JOINs this table to read `settings.appConfig`, which + // is the last link in the iOS/Android/web URL fallback chain (link → template + // → organization). Core therefore *depends* on the table existing even though + // richer deployments own the real one: without it every redirect fails with + // `relation "organizations" does not exist` (issue #35). + // + // Deliberately minimal — id and settings are all the redirect reads, plus + // suspended_at for the owner-restriction gate. CREATE TABLE IF NOT EXISTS is a + // no-op against a deployment that already ships a fuller organizations table, + // so this cannot clobber one. + await client.query(` + CREATE TABLE IF NOT EXISTS organizations ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + name VARCHAR(255), + settings JSONB DEFAULT '{}', + suspended_at TIMESTAMP, + created_at TIMESTAMP DEFAULT NOW(), + updated_at TIMESTAMP DEFAULT NOW() + ) + `); + // Link templates table (must be created before links, which references it) await client.query(` CREATE TABLE IF NOT EXISTS link_templates ( @@ -190,6 +213,24 @@ export async function initializeDatabase(options: DatabaseOptions = {}) { ) `); + // Add organization_id column to links table. + // + // The redirect join is `ON l.organization_id = o.id`, so the column is as + // load-bearing as the table itself. Nullable and unset by default: a core + // deployment that never populates it simply gets NULL org_settings and the + // fallback chain stops at the template level, exactly as before. + await client.query(` + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_name='links' AND column_name='organization_id' + ) THEN + ALTER TABLE links ADD COLUMN organization_id UUID REFERENCES organizations(id) ON DELETE SET NULL; + END IF; + END $$; + `); + // Add template_id column to links table await client.query(` DO $$ @@ -496,6 +537,7 @@ export async function initializeDatabase(options: DatabaseOptions = {}) { await client.query('CREATE UNIQUE INDEX IF NOT EXISTS idx_link_templates_slug ON link_templates(slug)'); await client.query('CREATE INDEX IF NOT EXISTS idx_link_templates_user_id ON link_templates(user_id)'); await client.query('CREATE INDEX IF NOT EXISTS idx_links_template_id ON links(template_id)'); + await client.query('CREATE INDEX IF NOT EXISTS idx_links_organization_id ON links(organization_id)'); // Indexes for webhooks await client.query('CREATE INDEX IF NOT EXISTS idx_webhooks_user_id ON webhooks(user_id)'); diff --git a/src/routes/health.test.ts b/src/routes/health.test.ts new file mode 100644 index 0000000..a5083ee --- /dev/null +++ b/src/routes/health.test.ts @@ -0,0 +1,94 @@ +/** + * Route-level tests for the health endpoints. + * + * The regression these lock down (issue #35): /health used to have no route at + * all, so it fell through to the catch-all redirect `/:shortCode` and was + * answered as a short-code lookup. The last test registers the real redirect + * plugin alongside health to prove the static path wins. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import { healthRoutes } from './health.js'; +import { redirectRoutes } from './redirect.js'; + +const query = vi.fn(); +vi.mock('../lib/database.js', () => ({ + db: { + query: (...args: unknown[]) => query(...args), + }, +})); + +let app: FastifyInstance; + +beforeEach(async () => { + query.mockReset(); + query.mockResolvedValue({ rows: [{ '?column?': 1 }], rowCount: 1 }); + app = Fastify(); + await app.register(healthRoutes); + await app.ready(); +}); + +afterEach(async () => { + await app.close(); +}); + +describe('GET /health', () => { + it('reports the process is up', async () => { + const res = await app.inject({ method: 'GET', url: '/health' }); + expect(res.statusCode).toBe(200); + expect(res.json()).toMatchObject({ status: 'ok' }); + }); + + it('never touches the database, so a database outage cannot fail liveness', async () => { + query.mockRejectedValue(new Error('connection refused')); + const res = await app.inject({ method: 'GET', url: '/health' }); + expect(res.statusCode).toBe(200); + expect(query).not.toHaveBeenCalled(); + }); +}); + +describe('GET /health/ready', () => { + it('is ready when the database answers', async () => { + const res = await app.inject({ method: 'GET', url: '/health/ready' }); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ status: 'ok', checks: { database: 'ok' } }); + }); + + it('is 503 when the database is unreachable', async () => { + query.mockRejectedValue(new Error('connection refused')); + const res = await app.inject({ method: 'GET', url: '/health/ready' }); + expect(res.statusCode).toBe(503); + expect(res.json()).toEqual({ status: 'error', checks: { database: 'error' } }); + }); + + it('reports a failing Redis as degraded, not unready', async () => { + const withRedis = Fastify(); + withRedis.decorate('redis', { ping: async () => { throw new Error('down'); } } as never); + await withRedis.register(healthRoutes); + await withRedis.ready(); + + const res = await withRedis.inject({ method: 'GET', url: '/health/ready' }); + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ status: 'ok', checks: { database: 'ok', redis: 'error' } }); + await withRedis.close(); + }); +}); + +describe('regression: /health is not swallowed by the redirect route', () => { + it('answers health, not a short-code lookup, when both plugins are registered', async () => { + const combined = Fastify(); + // Redirect first — the static route must win on specificity, not order. + await combined.register(redirectRoutes); + await combined.register(healthRoutes); + await combined.ready(); + + const res = await combined.inject({ method: 'GET', url: '/health' }); + expect(res.statusCode).toBe(200); + expect(res.json()).toMatchObject({ status: 'ok' }); + // The redirect handler would have run a links lookup; health must not. + const linkLookups = query.mock.calls.filter(([sql]) => /FROM links/i.test(String(sql))); + expect(linkLookups).toHaveLength(0); + + await combined.close(); + }); +}); diff --git a/src/routes/health.ts b/src/routes/health.ts new file mode 100644 index 0000000..26edd6c --- /dev/null +++ b/src/routes/health.ts @@ -0,0 +1,59 @@ +import { FastifyInstance } from 'fastify'; +import { db } from '../lib/database.js'; + +/** + * Health endpoints. + * + * Without these, `GET /health` fell through to the catch-all redirect route + * `/:shortCode` and was answered as a short-code lookup — a 404 at best, and on + * a self-hosted install a 500, which made the Docker HEALTHCHECK (which targets + * /health) permanently unhealthy and buried the real error (issue #35). + * + * Two levels, following the usual liveness/readiness split: + * + * /health — liveness. The process is up and serving. Never touches the + * database, so a database blip cannot get the container killed + * by an orchestrator that restarts on a failing probe. + * /health/ready — readiness. Confirms the database answers, and reports Redis + * when it is configured. 503 when the database is unreachable, + * so a load balancer can drain the instance. + * + * Both are static paths, which Fastify's router always prefers over the + * parametric `/:shortCode`, so they cannot be shadowed by a short link — and by + * the same token `health` is no longer usable as a short code. + */ +export async function healthRoutes(fastify: FastifyInstance) { + fastify.get('/health', async () => ({ + status: 'ok', + uptime: Math.floor(process.uptime()), + })); + + fastify.get('/health/ready', async (_request, reply) => { + const checks: Record = {}; + + try { + await db.query('SELECT 1'); + checks.database = 'ok'; + } catch (error) { + fastify.log.error(`Health: database check failed: ${error}`); + checks.database = 'error'; + } + + if (fastify.redis) { + try { + await fastify.redis.ping(); + checks.redis = 'ok'; + } catch { + // Redis is an optional cache with database fallback, so a failure here + // is degraded, not unready — it must not flip the overall status. + checks.redis = 'error'; + } + } + + const ready = checks.database === 'ok'; + return reply.status(ready ? 200 : 503).send({ + status: ready ? 'ok' : 'error', + checks, + }); + }); +} diff --git a/src/routes/index.ts b/src/routes/index.ts index d48bfcd..b58e3ff 100644 --- a/src/routes/index.ts +++ b/src/routes/index.ts @@ -8,3 +8,4 @@ export { templateRoutes } from './templates.js'; export { previewRoutes } from './preview.js'; export { debugRoutes } from './debug.js'; export { wellKnownRoutes } from './well-known.js'; +export { healthRoutes } from './health.js'; From 13f6cec1408dba31ea1af3fec091b5d3db3c547e Mon Sep 17 00:00:00 2001 From: Brandon Estrella Date: Mon, 10 Aug 2026 18:28:58 -0700 Subject: [PATCH 2/2] fix(docker): stop the `prepare` script from breaking the production install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docker build` failed at the production stage with exit code 127: > @linkforty/core@1.20.0 prepare > npm run build sh: tsc: not found package.json defines `prepare: npm run build`, which npm runs automatically on install. The production stage installs with devDependencies omitted, so tsc is absent and the implicit build dies — taking the whole image with it. Adds --ignore-scripts to both installs and switches the deprecated --only=production to --omit=dev. The builder stage does not need the implicit build either: it ran tsc before the source was even copied, and the real build is the explicit `npm run build` step that follows. Verified: image builds, and the container comes up healthy against Postgres and Redis — link creation, a 302 redirect through a short code, a 404 on an unknown code, and both health endpoints all behave. --- Dockerfile | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Dockerfile b/Dockerfile index 2eccc48..9e08b7f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,8 +6,10 @@ WORKDIR /app # Copy package files COPY package*.json ./ -# Install dependencies (including devDependencies for build) -RUN npm ci +# Install dependencies (including devDependencies for build). +# --ignore-scripts skips the `prepare` script, which would otherwise run tsc here, +# before the source is even copied. The real build is the explicit step below. +RUN npm ci --ignore-scripts # Copy source files COPY . . @@ -30,8 +32,11 @@ WORKDIR /app # Copy package files COPY package*.json ./ -# Install production dependencies only -RUN npm ci --only=production && \ +# Install production dependencies only. +# --ignore-scripts is required: package.json has a `prepare` script (npm run build) +# that npm runs automatically on install, and it needs tsc from devDependencies — +# which this stage deliberately omits. Without it the build dies with exit 127. +RUN npm ci --omit=dev --ignore-scripts && \ npm cache clean --force # Copy built files from builder