From 4955ee56cd3844cea864d8f007a103e40ecb8ac4 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Sun, 23 Aug 2026 09:51:11 -0400 Subject: [PATCH 1/2] fix(health): readiness probe returns 503 when the database is down readiness() caught the Terminus ServiceUnavailableException and returned a plain object, which NestJS serialized as HTTP 200 - so a pod that lost Postgres stayed Ready, kept taking traffic, and a maxUnavailable=0 rollout could replace both pods into a broken DB config successfully. Let the exception propagate so the probe sees the 503, and gate readiness on the database only: a Matrix outage degrades chat, not the API, and must not pull every api pod out of the Service endpoints (/health/matrix still carries that signal). Spec: the disconnected case now expects propagation instead of an error body, plus HTTP-level tests through the real Terminus pipeline - the old behavior (200 with the DB ping failing) was observed failing before the fix. Refs: om-ep1ej --- src/health/health.controller.spec.ts | 113 ++++++++++++++++++++++----- src/health/health.controller.ts | 17 ++-- 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/src/health/health.controller.spec.ts b/src/health/health.controller.spec.ts index a4bd84f2..3304cd7d 100644 --- a/src/health/health.controller.spec.ts +++ b/src/health/health.controller.spec.ts @@ -1,8 +1,12 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { INestApplication } from '@nestjs/common'; +import request from 'supertest'; import { HealthController } from './health.controller'; import { + HealthCheckError, HealthCheckService, HttpHealthIndicator, + TerminusModule, TypeOrmHealthIndicator, } from '@nestjs/terminus'; import { MatrixHealthIndicator } from '../matrix/health/matrix.health'; @@ -10,11 +14,28 @@ import { MatrixHealthIndicator } from '../matrix/health/matrix.health'; describe('HealthController', () => { let controller: HealthController; let healthCheckService: HealthCheckService; + let dbIndicator: { pingCheck: jest.Mock }; + let matrixIndicator: { isHealthy: jest.Mock }; beforeEach(async () => { healthCheckService = { check: jest.fn(), } as any; + dbIndicator = { + pingCheck: jest.fn().mockResolvedValue({ database: { status: 'up' } }), + }; + matrixIndicator = { + isHealthy: jest.fn().mockResolvedValue({ + status: 'up', + matrix: { + serverAvailable: true, + tokenState: 'valid', + tokenValid: true, + adminPrivilegesValid: true, + serverUrl: 'https://matrix.example.org', + }, + }), + }; const module: TestingModule = await Test.createTestingModule({ controllers: [HealthController], @@ -29,22 +50,11 @@ describe('HealthController', () => { }, { provide: TypeOrmHealthIndicator, - useValue: {}, + useValue: dbIndicator, }, { provide: MatrixHealthIndicator, - useValue: { - isHealthy: jest.fn().mockResolvedValue({ - status: 'up', - matrix: { - serverAvailable: true, - tokenState: 'valid', - tokenValid: true, - adminPrivilegesValid: true, - serverUrl: 'https://matrix.example.org', - }, - }), - }, + useValue: matrixIndicator, }, ], }).compile(); @@ -68,16 +78,31 @@ describe('HealthController', () => { }); }); - it('should return status error when database is disconnected', async () => { - const error = new Error('Database connection failed'); + it('should propagate the ServiceUnavailableException when the database is disconnected, so the probe sees a 503', async () => { + // health.check() throws ServiceUnavailableException on a failed check; + // readiness must let it reach the HTTP layer instead of returning a 200 body. + const error = new Error('Health Check has failed!'); jest.spyOn(healthCheckService, 'check').mockRejectedValue(error); - const result = await controller.readiness(); - expect(result).toEqual({ - status: 'error', - database: 'disconnected', - error: error.message, - }); + await expect(controller.readiness()).rejects.toBe(error); + }); + + it('should gate on the database only — matrix is not part of readiness', async () => { + // Decision (TS, 2026-08-23): a Matrix outage degrades chat, not the API, + // and must not pull api pods out of the Service endpoints. + jest + .spyOn(healthCheckService, 'check') + .mockImplementation(async (checks) => { + for (const check of checks) { + await check(); + } + return { status: 'ok' } as any; + }); + + await controller.readiness(); + + expect(dbIndicator.pingCheck).toHaveBeenCalledWith('database'); + expect(matrixIndicator.isHealthy).not.toHaveBeenCalled(); }); }); @@ -107,3 +132,49 @@ describe('HealthController', () => { }); }); }); + +describe('HealthController (HTTP, real Terminus check pipeline)', () => { + let app: INestApplication; + const dbIndicator = { pingCheck: jest.fn() }; + + beforeAll(async () => { + const module: TestingModule = await Test.createTestingModule({ + imports: [TerminusModule], + controllers: [HealthController], + providers: [ + { provide: HttpHealthIndicator, useValue: {} }, + { provide: TypeOrmHealthIndicator, useValue: dbIndicator }, + { provide: MatrixHealthIndicator, useValue: { isHealthy: jest.fn() } }, + ], + }).compile(); + + app = module.createNestApplication({ logger: false }); + await app.init(); + }); + + afterAll(async () => { + await app.close(); + }); + + it('should answer 503 when the database ping fails', async () => { + dbIndicator.pingCheck.mockRejectedValue( + new HealthCheckError('TypeOrm health check failed', { + database: { status: 'down' }, + }), + ); + + const res = await request(app.getHttpServer()).get('/health/readiness'); + + expect(res.status).toBe(503); + expect(res.body.status).toBe('error'); + }); + + it('should answer 200 when the database ping succeeds', async () => { + dbIndicator.pingCheck.mockResolvedValue({ database: { status: 'up' } }); + + const res = await request(app.getHttpServer()).get('/health/readiness'); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('ok'); + }); +}); diff --git a/src/health/health.controller.ts b/src/health/health.controller.ts index cc90c2ab..84a9f47b 100644 --- a/src/health/health.controller.ts +++ b/src/health/health.controller.ts @@ -38,18 +38,11 @@ export class HealthController { @Get('readiness') @ApiOperation({ summary: 'Readiness probe' }) async readiness() { - try { - return await this.health.check([ - () => this.db.pingCheck('database'), - () => this.matrix.isHealthy('matrix'), - ]); - } catch (error) { - return { - status: 'error', - database: 'disconnected', - error: error.message, - }; - } + // Let the ServiceUnavailableException propagate: the 503 is how the probe + // learns the pod cannot serve, so Kubernetes pulls it from the Service. + // Database only — a Matrix outage degrades chat, not the API, and must not + // take every api pod out of rotation (see /health/matrix for that signal). + return await this.health.check([() => this.db.pingCheck('database')]); } @HealthCheck() From d6218dce45eb042898be7aa616780bd4cb1ace76 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Sun, 23 Aug 2026 09:58:27 -0400 Subject: [PATCH 2/2] test(health): run the readiness HTTP spec through GlobalExceptionFilter The app registers GlobalExceptionFilter via APP_FILTER (interceptors.module.ts), so in production it catches the propagated ServiceUnavailableException, keeps the 503, and rewrites the body to {statusCode, message, path, timestamp}. Mirror that in the HTTP-level spec so the asserted response is the one the probes actually see. --- src/health/health.controller.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/health/health.controller.spec.ts b/src/health/health.controller.spec.ts index 3304cd7d..764e146c 100644 --- a/src/health/health.controller.spec.ts +++ b/src/health/health.controller.spec.ts @@ -10,6 +10,7 @@ import { TypeOrmHealthIndicator, } from '@nestjs/terminus'; import { MatrixHealthIndicator } from '../matrix/health/matrix.health'; +import { GlobalExceptionFilter } from '../filters/global-exception.filter'; describe('HealthController', () => { let controller: HealthController; @@ -149,6 +150,9 @@ describe('HealthController (HTTP, real Terminus check pipeline)', () => { }).compile(); app = module.createNestApplication({ logger: false }); + // Mirror production: GlobalExceptionFilter (APP_FILTER in InterceptorsModule) + // catches the ServiceUnavailableException, keeps its 503, and rewrites the body. + app.useGlobalFilters(new GlobalExceptionFilter({ inc: jest.fn() } as any)); await app.init(); }); @@ -166,7 +170,8 @@ describe('HealthController (HTTP, real Terminus check pipeline)', () => { const res = await request(app.getHttpServer()).get('/health/readiness'); expect(res.status).toBe(503); - expect(res.body.status).toBe('error'); + expect(res.body.statusCode).toBe(503); + expect(res.body.path).toBe('/health/readiness'); }); it('should answer 200 when the database ping succeeds', async () => {