From 147417d7d4659e1c6ab34eb10c3c75e9bc41cf27 Mon Sep 17 00:00:00 2001 From: Nick Szaunig Date: Tue, 25 Aug 2026 23:07:29 +0200 Subject: [PATCH] fix: address code-review findings (startup validation, shutdown hooks, health check, deploy config) Verified each finding against the real codebase first, since the review's diff was polluted by a stale leftover agent-worktree copy under .claude/worktrees/ (now removed) that made several already-fixed or never-broken issues look current (CORS, missing typecheck script, PWA icons, font CSS, Helmet/Swagger CSP - all confirmed fine live, not touched). Real fixes: - main.ts: fail-fast on missing DATABASE_URL/JWT_ACCESS_SECRET at boot, call enableShutdownHooks() so PrismaService.onModuleDestroy() actually runs on SIGTERM during k8s rolling deploys, PORT fallback 3000 -> 3001 to match every other place that documents the port. - health.controller.ts: log the real underlying DB error instead of discarding it, add a 5s timeout to the readiness query so a hung DB fails the probe fast instead of blocking indefinitely. - Frontend Dockerfile: declare ARG/ENV NEXT_PUBLIC_API_URL before the build step, since Next.js inlines NEXT_PUBLIC_* vars at build time and a runtime-only ConfigMap value never reaches the compiled bundle. - Backend ingress.yaml: drop the no-op rewrite-target regex for a plain prefix path (identical behavior, less confusing). --- .../src/health/health.controller.ts | 28 +++++++++++++++++-- ApiCore-Backend/src/main.ts | 16 ++++++++++- ApiCore-Frontend/Dockerfile | 6 ++++ deploy/k8s/backend/ingress.yaml | 5 ++-- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/ApiCore-Backend/src/health/health.controller.ts b/ApiCore-Backend/src/health/health.controller.ts index 2c9d664..c904318 100644 --- a/ApiCore-Backend/src/health/health.controller.ts +++ b/ApiCore-Backend/src/health/health.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get, + Logger, ServiceUnavailableException, VERSION_NEUTRAL, } from '@nestjs/common'; @@ -8,10 +9,29 @@ import { ApiTags } from '@nestjs/swagger'; import { PrismaService } from '../prisma/prisma.service'; import { Public } from '../auth/decorators/public.decorator'; +const READINESS_QUERY_TIMEOUT_MS = 5000; + +async function withTimeout(promise: Promise, ms: number): Promise { + let timer: NodeJS.Timeout; + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error('Readiness query timed out')), + ms, + ); + }); + try { + return await Promise.race([promise, timeoutPromise]); + } finally { + clearTimeout(timer!); + } +} + @ApiTags('health') @Public() @Controller({ path: 'health', version: VERSION_NEUTRAL }) export class HealthController { + private readonly logger = new Logger(HealthController.name); + constructor(private readonly prisma: PrismaService) {} @Get() @@ -22,9 +42,13 @@ export class HealthController { @Get('ready') async readiness(): Promise<{ status: 'ok' }> { try { - await this.prisma.$queryRaw`SELECT 1`; + await withTimeout( + this.prisma.$queryRaw`SELECT 1`, + READINESS_QUERY_TIMEOUT_MS, + ); return { status: 'ok' }; - } catch { + } catch (error: unknown) { + this.logger.error('Readiness check failed', error); throw new ServiceUnavailableException('Database not reachable'); } } diff --git a/ApiCore-Backend/src/main.ts b/ApiCore-Backend/src/main.ts index 01e2b23..3863eaf 100644 --- a/ApiCore-Backend/src/main.ts +++ b/ApiCore-Backend/src/main.ts @@ -4,8 +4,22 @@ import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; import { AppModule } from './app.module'; import { configureApp } from './app.config'; +const REQUIRED_ENV_VARS = ['DATABASE_URL', 'JWT_ACCESS_SECRET'] as const; + +function assertRequiredEnvVars(): void { + const missing = REQUIRED_ENV_VARS.filter((name) => !process.env[name]); + if (missing.length > 0) { + throw new Error( + `Missing required environment variable(s): ${missing.join(', ')}`, + ); + } +} + async function bootstrap() { + assertRequiredEnvVars(); + const app = await NestFactory.create(AppModule); + app.enableShutdownHooks(); configureApp(app); @@ -18,6 +32,6 @@ async function bootstrap() { const swaggerDocument = SwaggerModule.createDocument(app, swaggerConfig); SwaggerModule.setup('api/v1/docs', app, swaggerDocument); - await app.listen(process.env.PORT ?? 3000); + await app.listen(process.env.PORT ?? 3001); } void bootstrap(); diff --git a/ApiCore-Frontend/Dockerfile b/ApiCore-Frontend/Dockerfile index f84b787..a8fbcc7 100644 --- a/ApiCore-Frontend/Dockerfile +++ b/ApiCore-Frontend/Dockerfile @@ -2,6 +2,12 @@ FROM node:22-alpine AS builder WORKDIR /app +# NEXT_PUBLIC_* vars are inlined into the client bundle at build time, so +# this must be supplied per-environment via: +# docker build --build-arg NEXT_PUBLIC_API_URL=https://api.example.com . +# Setting it only in the runtime ConfigMap has no effect on the built bundle. +ARG NEXT_PUBLIC_API_URL +ENV NEXT_PUBLIC_API_URL=$NEXT_PUBLIC_API_URL COPY package.json package-lock.json ./ RUN npm ci COPY . . diff --git a/deploy/k8s/backend/ingress.yaml b/deploy/k8s/backend/ingress.yaml index 9600a5b..ca1e39d 100644 --- a/deploy/k8s/backend/ingress.yaml +++ b/deploy/k8s/backend/ingress.yaml @@ -3,7 +3,6 @@ kind: Ingress metadata: name: apicore-backend annotations: - nginx.ingress.kubernetes.io/rewrite-target: /$2 # Requires cert-manager with a ClusterIssuer named "letsencrypt-prod" # (or swap for whatever issuer/cert mechanism the cluster already uses). cert-manager.io/cluster-issuer: letsencrypt-prod @@ -17,8 +16,8 @@ spec: - host: api.apicore.example.com http: paths: - - path: /()(.*) - pathType: ImplementationSpecific + - path: / + pathType: Prefix backend: service: name: apicore-backend