diff --git a/apps/vps/src/app.ts b/apps/vps/src/app.ts index 8299426a..37a2cf0b 100644 --- a/apps/vps/src/app.ts +++ b/apps/vps/src/app.ts @@ -1,6 +1,4 @@ -import { sql } from 'drizzle-orm' -import { Data, Duration, Effect, Schedule } from 'effect' -import type { Context } from 'hono' +import { Duration, Effect, Schedule } from 'effect' import configureOpenAPI from '@/lib/configure-open-api' import { createAppEffect } from '@/lib/create-app' import admin from '@/routes/admin/admin.index' @@ -22,25 +20,12 @@ import spotify from '@/routes/spotify/spotify.index' import upload from '@/routes/upload/upload.index' import uploadMultipart from '@/routes/upload-multipart/upload-multipart.index' import user from '@/routes/user/user.index' -import { db } from './db' import { regenerateSitemap } from './routes/redirect/seo/sitemap' import { runApp, runAppFork } from './runtime' import { processPendingReminders, queryNextDueReminder } from './services/reminder-processor' import { ReminderSignalService } from './services/reminder-signal.service' import { SentryService } from './services/sentry.service' -class HealthCheckError extends Data.TaggedError('HealthCheckError')<{ - cause?: unknown -}> {} - -const healthCheckEffect = Effect.tryPromise({ - try: () => db.execute(sql.raw('SELECT 1')), - catch: (cause) => new HealthCheckError({ cause }) -}) - -const READINESS_CACHE_MS = 5_000 -let readinessCache: { checkedAt: number; status: 200 | 500 } | undefined - const setupRoutesEffect = Effect.gen(function* () { yield* SentryService const app = yield* createAppEffect @@ -66,38 +51,12 @@ const setupRoutesEffect = Effect.gen(function* () { app.route('/api/music-reminders', musicReminders) // Kept at root, not under /api: these are externally-referenced public URLs. - // /auth is handled by the Effect router directly (apps/vps/src/http/routes.ts, - // step 2c) -- not mounted here. + // /auth and /health are handled by the Effect router directly + // (apps/vps/src/http/routes.ts, steps 2c/3a) -- not mounted here. app.route('/s', shareRouter) app.route('', rss) app.route('', seoRouter) - app.get('/health/live', (c) => c.json({ ok: true }, 200)) - - const readinessHealthRoute = async (c: Context) => { - const cache = readinessCache - const cachedStatus = cache && Date.now() - cache.checkedAt < READINESS_CACHE_MS - - if (cachedStatus) { - return c.json({ dbConnected: cache.status === 200 }, cache.status) - } - - const program = healthCheckEffect.pipe( - Effect.map(() => ({ data: { dbConnected: true }, status: 200 as const })), - Effect.catch(() => Effect.succeed({ data: { dbConnected: false }, status: 500 as const })) - ) - - const result = await runApp(program) - readinessCache = { - checkedAt: Date.now(), - status: result.status - } - return c.json(result.data, result.status) - } - - app.get('/health/ready', readinessHealthRoute) - app.get('/health', readinessHealthRoute) - return app }) diff --git a/apps/vps/src/health.blackbox.test.ts b/apps/vps/src/health.blackbox.test.ts deleted file mode 100644 index e06a7016..00000000 --- a/apps/vps/src/health.blackbox.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { HealthLiveResponse, HealthReadyResponse } from '@gbfm/api/health' -import { decodeResponseBody } from '@gbfm/api/testing' -import { beforeAll, describe, expect, it } from 'vitest' -import type { AppType } from '@/app' - -// Asserts only the wire contract (status, JSON body against @gbfm/api schemas) so these same assertions can run unchanged once health is ported to HttpApiBuilder (step 3a). -let app: AppType - -beforeAll(async () => { - const mod = await import('@/app') - app = mod.default -}) - -describe('Health endpoints', () => { - it('GET /health/live returns 200 with liveness JSON', async () => { - const res = await app.request('/health/live') - - expect(res.status).toBe(200) - await expect(decodeResponseBody(HealthLiveResponse, res)).resolves.toEqual({ ok: true }) - }) - - it('GET /health/ready returns 200 with readiness JSON when the database is reachable', async () => { - const res = await app.request('/health/ready') - - expect(res.status).toBe(200) - await expect(decodeResponseBody(HealthReadyResponse, res)).resolves.toEqual({ - dbConnected: true - }) - }) - - it('GET /health returns the same readiness result as /health/ready', async () => { - const res = await app.request('/health') - - expect(res.status).toBe(200) - await expect(decodeResponseBody(HealthReadyResponse, res)).resolves.toEqual({ - dbConnected: true - }) - }) - - it('repeated readiness calls within the cache window keep returning 200', async () => { - const first = await app.request('/health/ready') - const second = await app.request('/health/ready') - - expect(first.status).toBe(200) - expect(second.status).toBe(200) - }) - - it('responds 404 to unsupported methods on health paths', async () => { - const res = await app.request('/health/live', { method: 'POST' }) - - expect(res.status).toBe(404) - }) -}) diff --git a/apps/vps/src/http/health.handlers.failure.test.ts b/apps/vps/src/http/health.handlers.failure.test.ts new file mode 100644 index 00000000..e6b48771 --- /dev/null +++ b/apps/vps/src/http/health.handlers.failure.test.ts @@ -0,0 +1,71 @@ +import { ReadinessCheckFailedError } from '@gbfm/api/errors' +import { decodeResponseBody } from '@gbfm/api/testing' +import { Effect } from 'effect' +import { describe, expect, it } from 'vitest' +import type { AppType } from '@/app' +import { createWebHandler } from './routes' + +// Separate file (docs/migration-effect-http-api.md, step 3a): each +// createWebHandler builds its own cached readiness check (health.handlers.ts, +// Effect.cachedWithTTL inside makeHealthHandlers), so this doesn't strictly +// need isolation from routes.blackbox.test.ts anymore -- kept in its own file +// as a dedicated home for the failure/concurrency paths. +describe('health readiness failure + cache', () => { + it('caches a failing readiness check and does not re-run it within the window', async () => { + const mod = await import('@/app') + const app: AppType = mod.default + + let checks = 0 + const scoped = createWebHandler(app, { + healthDatabaseCheck: Effect.sync(() => { + checks += 1 + }).pipe( + Effect.flatMap(() => Effect.fail(new ReadinessCheckFailedError({ dbConnected: false }))) + ) + }) + + try { + const first = await scoped.handler(new Request('http://localhost/health/ready')) + const second = await scoped.handler(new Request('http://localhost/health/ready')) + + expect(first.status).toBe(500) + expect(second.status).toBe(500) + + const decoded = await decodeResponseBody(ReadinessCheckFailedError, first) + expect(decoded._tag).toBe('ReadinessCheckFailedError') + expect(decoded.dbConnected).toBe(false) + + // Second call must reuse the cached failure, not re-run the check. + expect(checks).toBe(1) + } finally { + await scoped.dispose() + } + }) + + it('concurrent requests on a cold cache share one in-flight check, not one each', async () => { + const mod = await import('@/app') + const app: AppType = mod.default + + let checks = 0 + const scoped = createWebHandler(app, { + healthDatabaseCheck: Effect.sync(() => { + checks += 1 + }).pipe(Effect.delay('20 millis')) + }) + + try { + const [first, second] = await Promise.all([ + scoped.handler(new Request('http://localhost/health/ready')), + scoped.handler(new Request('http://localhost/health/ready')) + ]) + + expect(first.status).toBe(200) + expect(second.status).toBe(200) + // Both requests arrived before the (delayed) check completed -- they + // must share the one in-flight computation, not race to run it twice. + expect(checks).toBe(1) + } finally { + await scoped.dispose() + } + }) +}) diff --git a/apps/vps/src/http/health.handlers.ts b/apps/vps/src/http/health.handlers.ts new file mode 100644 index 00000000..b12f141d --- /dev/null +++ b/apps/vps/src/http/health.handlers.ts @@ -0,0 +1,60 @@ +import { Api } from '@gbfm/api/api' +import { ReadinessCheckFailedError } from '@gbfm/api/errors' +import { sql } from 'drizzle-orm' +import { Effect, Layer } from 'effect' +import { HttpApiBuilder } from 'effect/unstable/httpapi' +import { db } from '@/db' + +const READINESS_CACHE_MS = 5_000 + +// Injectable so tests can force the failure path without a real DB outage +// (docs/migration-effect-http-api.md, step 1b noted this gap; step 3a closes it). +// The cause is logged server-side only -- it must never reach the response +// body (a public /health endpoint leaking internal DB error detail is a real +// information-disclosure risk), but it also must not be silently discarded. +export const checkDatabase = Effect.tryPromise({ + try: () => db.execute(sql.raw('SELECT 1')), + catch: (cause) => cause +}).pipe( + Effect.tapError((cause) => Effect.logError('[health] readiness check failed', cause)), + Effect.mapError(() => new ReadinessCheckFailedError({ dbConnected: false })), + Effect.asVoid +) + +type ReadinessResult = { readonly dbConnected: true } | { readonly dbConnected: false } + +// Effect.cachedWithTTL memoizes the *pending* computation, not just its result: +// concurrent requests that arrive while a check is in flight share that one +// fiber instead of each independently hitting the database and racing to +// write a cache slot (the module-level `let` this replaced had that race). +const readinessResult = (check: Effect.Effect) => + check.pipe( + Effect.as({ dbConnected: true }), + Effect.catch(() => Effect.succeed({ dbConnected: false })), + Effect.cachedWithTTL(`${READINESS_CACHE_MS} millis`) + ) + +const readiness = (cachedCheck: Effect.Effect) => + Effect.gen(function* () { + const result = yield* cachedCheck + if (!result.dbConnected) { + return yield* new ReadinessCheckFailedError({ dbConnected: false }) + } + return result + }) + +export const makeHealthHandlers = (check: Effect.Effect) => + Layer.unwrap( + Effect.gen(function* () { + const cachedCheck = yield* readinessResult(check) + + return HttpApiBuilder.group(Api, 'health', (handlers) => + handlers + .handle('live', () => Effect.succeed({ ok: true as const })) + .handle('ready', () => readiness(cachedCheck)) + .handle('check', () => readiness(cachedCheck)) + ) + }) + ) + +export const HealthHandlersLive = makeHealthHandlers(checkDatabase) diff --git a/apps/vps/src/http/routes.blackbox.test.ts b/apps/vps/src/http/routes.blackbox.test.ts index 51635d5d..cc2685a5 100644 --- a/apps/vps/src/http/routes.blackbox.test.ts +++ b/apps/vps/src/http/routes.blackbox.test.ts @@ -4,9 +4,9 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest' import type { AppType } from '@/app' import { createWebHandler } from './routes' -// Step 2a (docs/migration-effect-http-api.md): the Effect toWebHandler + Hono -// fallback must behave identically to the plain Hono app it wraps. Reuses the -// same @gbfm/api schemas as the 1b blackbox suite so both point at one contract. +// Blackbox suite asserting only the wire contract, so these assertions keep +// working as more groups move from the Hono fallback onto the Effect router +// (docs/migration-effect-http-api.md). let app: AppType let webHandler: ReturnType @@ -21,22 +21,6 @@ afterAll(async () => { }) describe('Effect toWebHandler fallback', () => { - it('GET /health/live matches the Hono app', async () => { - const res = await webHandler.handler(new Request('http://localhost/health/live')) - - expect(res.status).toBe(200) - await expect(decodeResponseBody(HealthLiveResponse, res)).resolves.toEqual({ ok: true }) - }) - - it('GET /health/ready matches the Hono app', async () => { - const res = await webHandler.handler(new Request('http://localhost/health/ready')) - - expect(res.status).toBe(200) - await expect(decodeResponseBody(HealthReadyResponse, res)).resolves.toEqual({ - dbConnected: true - }) - }) - it('GET /api/music/artists returns the same response as the plain Hono app', async () => { const [viaHandler, viaHono] = await Promise.all([ webHandler.handler(new Request('http://localhost/api/music/artists')), @@ -75,3 +59,42 @@ describe('better-auth route (Step 2c)', () => { expect(await withAuth.text()).not.toEqual(await withoutAuth.text()) }) }) + +describe('health (HttpApiBuilder group, Step 3a)', () => { + it('GET /health/live returns 200 without checking the database', async () => { + const res = await webHandler.handler(new Request('http://localhost/health/live')) + + expect(res.status).toBe(200) + await expect(decodeResponseBody(HealthLiveResponse, res)).resolves.toEqual({ ok: true }) + }) + + it('GET /health/ready and /health return 200 when the database check succeeds', async () => { + const res = await webHandler.handler(new Request('http://localhost/health/ready')) + const checkRes = await webHandler.handler(new Request('http://localhost/health')) + + expect(res.status).toBe(200) + expect(checkRes.status).toBe(200) + await expect(decodeResponseBody(HealthReadyResponse, res)).resolves.toEqual({ + dbConnected: true + }) + await expect(decodeResponseBody(HealthReadyResponse, checkRes)).resolves.toEqual({ + dbConnected: true + }) + }) + + it('repeated readiness calls within the cache window keep returning 200', async () => { + const first = await webHandler.handler(new Request('http://localhost/health/ready')) + const second = await webHandler.handler(new Request('http://localhost/health/ready')) + + expect(first.status).toBe(200) + expect(second.status).toBe(200) + }) + + it('responds 404 to unsupported methods on health paths', async () => { + const res = await webHandler.handler( + new Request('http://localhost/health/live', { method: 'POST' }) + ) + + expect(res.status).toBe(404) + }) +}) diff --git a/apps/vps/src/http/routes.ts b/apps/vps/src/http/routes.ts index f9556ef0..91019142 100644 --- a/apps/vps/src/http/routes.ts +++ b/apps/vps/src/http/routes.ts @@ -1,8 +1,13 @@ +import { Api } from '@gbfm/api/api' +import type { ReadinessCheckFailedError } from '@gbfm/api/errors' import { Effect, Layer } from 'effect' import { HttpRouter, HttpServer, HttpServerRequest, HttpServerResponse } from 'effect/unstable/http' +import { HttpApiBuilder } from 'effect/unstable/httpapi' import type { AppType } from '@/app' +import { checkDatabase, makeHealthHandlers } from '@/http/health.handlers' import { auth } from '@/lib/auth' import { prepareAuthRequest } from '@/routes/user/better-auth.routes' +import { AppLoggerLive } from '@/services/logger.service' // Routes every request to the existing Hono app unchanged. Removed one HttpApi group at a time as routes are ported (docs/migration-effect-http-api.md). export const honoFallback = (honoApp: AppType) => @@ -26,7 +31,24 @@ const betterAuthRoute = HttpRouter.add('*', '/auth/*', (request) => }) ) -export const createWebHandler = (honoApp: AppType) => - HttpRouter.toWebHandler( - Layer.mergeAll(betterAuthRoute, honoFallback(honoApp), HttpServer.layerServices) +// Step 3a (docs/migration-effect-http-api.md): first real HttpApi group taking +// over live traffic from the Hono fallback. Test seams accept an alternate +// database check so tests can force the failure/cache paths. +export const createWebHandler = ( + honoApp: AppType, + options?: { readonly healthDatabaseCheck?: Effect.Effect } +) => { + const HealthLive = HttpApiBuilder.layer(Api).pipe( + Layer.provide(makeHealthHandlers(options?.healthDatabaseCheck ?? checkDatabase)) ) + + return HttpRouter.toWebHandler( + Layer.mergeAll(HealthLive, betterAuthRoute, honoFallback(honoApp)).pipe( + Layer.provideMerge(HttpServer.layerServices), + // Effect.logError inside health handlers must reach the app's real + // Pino + Sentry logger, not Effect's bare default console logger -- + // otherwise a DB outage's cause is logged nowhere on-call looks. + Layer.provide(AppLoggerLive) + ) + ) +} diff --git a/packages/api/package.json b/packages/api/package.json index 6a83b711..31e579e1 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -3,6 +3,7 @@ "version": "0.0.1", "type": "module", "exports": { + "./api": "./src/api.ts", "./health": "./src/health.ts", "./errors": "./src/errors.ts", "./testing": "./src/testing.ts" diff --git a/packages/api/src/api.ts b/packages/api/src/api.ts new file mode 100644 index 00000000..75c9f947 --- /dev/null +++ b/packages/api/src/api.ts @@ -0,0 +1,4 @@ +import { HttpApi } from 'effect/unstable/httpapi' +import { HealthGroup } from './health' + +export const Api = HttpApi.make('gbfm').add(HealthGroup)