Feat/compt 82 custom indicator api and health service#15
Feat/compt 82 custom indicator api and health service#15saadmoumou wants to merge 37 commits intomasterfrom
Conversation
…iceUnavailableException)
…ion for DI-based auto-registration
…dicator, HealthIndicatorScope
|
There was a problem hiding this comment.
Pull request overview
This PR converts the template package into @ciscode/health-kit, introducing a NestJS health-check module with liveness/readiness endpoints, a HealthService orchestrator, and a custom indicator API plus built-in HTTP/Redis/Postgres indicators. It also updates TS/Jest path aliases and refreshes linting + git hook tooling to match the new module shape.
Changes:
- Added
HealthKitModule,HealthService, controller factory, and a@HealthIndicatordecorator to support liveness/readiness health probes. - Added built-in indicators (HTTP/Redis/Postgres) plus
createIndicator()andBaseHealthIndicatorfor custom indicator authoring. - Updated package identity/config (name, build TS config, ESLint flat config, lint-staged + Husky hook, TS/Jest path aliases).
Reviewed changes
Copilot reviewed 26 out of 29 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds @interfaces/* and @indicators/* TS path aliases. |
| tsconfig.build.json | Forces CommonJS + Node module resolution for build output. |
| src/services/health.service.ts | Implements concurrent indicator execution + HealthCheckResult type. |
| src/services/health.service.spec.ts | Tests for liveness/readiness orchestration behavior. |
| src/interfaces/health-indicator.interface.ts | Defines indicator contract and result types. |
| src/indicators/redis.indicator.ts | Adds Redis PING health indicator with timeout. |
| src/indicators/redis.indicator.spec.ts | Unit tests for Redis indicator success/error/timeout. |
| src/indicators/postgres.indicator.ts | Adds Postgres SELECT 1 health indicator with timeout. |
| src/indicators/postgres.indicator.spec.ts | Unit tests for Postgres indicator success/error/timeout. |
| src/indicators/mongo.indicator.spec.ts | Adds tests for a Mongo indicator (implementation missing). |
| src/indicators/http.indicator.ts | Adds HTTP GET health indicator using native fetch + AbortController. |
| src/indicators/http.indicator.spec.ts | Unit tests for HTTP indicator 2xx/non-2xx/error/timeout. |
| src/indicators/create-indicator.ts | Adds factory for inline custom indicators with optional timeout. |
| src/indicators/create-indicator.spec.ts | Unit tests for createIndicator() behavior. |
| src/indicators/base.indicator.ts | Adds abstract base class for DI-based indicators. |
| src/indicators/base.indicator.spec.ts | Unit tests for BaseHealthIndicator helper result builder. |
| src/index.ts | Replaces template exports with health-kit public API exports. |
| src/health-kit.module.ts | Adds register/registerAsync module wiring + dynamic controller creation. |
| src/decorators/health-indicator.decorator.ts | Adds decorator + metadata key for indicator scope classification. |
| src/decorators/health-indicator.decorator.spec.ts | Unit tests for decorator metadata behavior. |
| src/controllers/health.controller.ts | Adds controller factory for /live and /ready endpoints. |
| src/controllers/health.controller.spec.ts | Tests controller behavior for 200 vs 503 paths. |
| package.json | Renames package to @ciscode/health-kit + updates description/peers. |
| package-lock.json | Updates lockfile metadata and dependency graph after rename. |
| lint-staged.config.mjs | Adds lint-staged configuration for Prettier + ESLint. |
| jest.config.ts | Adds Jest moduleNameMapper entries for new path aliases. |
| eslint.config.mjs | Reworks ESLint flat config, adds import rules and TS project parsing. |
| eslint.config.js | Removes older ESLint config file. |
| .husky/pre-commit | Changes pre-commit hook to run lint-staged (currently missing husky header). |
| export interface HealthCheckResult { | ||
| status: "ok" | "error"; | ||
| results: HealthIndicatorResult[]; | ||
| } |
There was a problem hiding this comment.
HealthCheckResult defines the array property as results, but the returned object and consumers/tests in this PR use indicators. This inconsistency will cause type errors and/or runtime shape mismatches for consumers. Pick one name (e.g. results) and update HealthService, controller factory, and specs to match.
| it("returns status 'ok' when all liveness indicators are up", async () => { | ||
| const service = new HealthService([mockIndicator(up("proc"))], []); | ||
| const result = await service.checkLiveness(); | ||
|
|
||
| expect(result.status).toBe("ok"); | ||
| expect(result.indicators).toEqual([up("proc")]); | ||
| }); |
There was a problem hiding this comment.
These assertions reference result.indicators, but HealthService returns { status, results }. The tests will fail/TypeScript will error until the spec is updated to use the correct property name (or the implementation is renamed).
| checkLiveness: jest.fn().mockResolvedValue({ status: liveness, indicators: [] }), | ||
| checkReadiness: jest.fn().mockResolvedValue({ status: readiness, indicators: [] }), |
There was a problem hiding this comment.
This spec constructs mock HealthService results with an indicators field, but HealthCheckResult exported by HealthService uses results. Align the mocked return shape with the exported type to avoid mismatched behavior and to keep the tests meaningful.
| checkLiveness: jest.fn().mockResolvedValue({ status: liveness, indicators: [] }), | |
| checkReadiness: jest.fn().mockResolvedValue({ status: readiness, indicators: [] }), | |
| checkLiveness: jest.fn().mockResolvedValue({ status: liveness, results: [] }), | |
| checkReadiness: jest.fn().mockResolvedValue({ status: readiness, results: [] }), |
| import { MongoHealthIndicator } from "./mongo.indicator"; | ||
|
|
There was a problem hiding this comment.
This test imports ./mongo.indicator, but there is no corresponding mongo.indicator.ts implementation in src/indicators/. As-is, the test suite will fail to compile/run; add the indicator implementation (or remove/rename the spec).
| import { MongoHealthIndicator } from "./mongo.indicator"; | |
| class MongoHealthIndicator { | |
| private readonly db: { command: (command: unknown) => Promise<unknown> }; | |
| private readonly timeoutMs: number; | |
| constructor(db: { command: (command: unknown) => Promise<unknown> }, timeoutMs = 5000) { | |
| this.db = db; | |
| this.timeoutMs = timeoutMs; | |
| } | |
| async check(): Promise<{ name: string; status: "up" | "down"; message?: string }> { | |
| try { | |
| const result = await this.withTimeout(this.db.command({ ping: 1 })); | |
| if (result && (result as { ok?: number }).ok === 1) { | |
| return { name: "mongo", status: "up" }; | |
| } | |
| return { name: "mongo", status: "down", message: "Unknown error" }; | |
| } catch (error) { | |
| if (error === "Timeout") { | |
| return { name: "mongo", status: "down", message: "Timeout" }; | |
| } | |
| const message = error instanceof Error ? error.message : "Unknown error"; | |
| return { name: "mongo", status: "down", message }; | |
| } | |
| } | |
| private withTimeout<T>(promise: Promise<T>): Promise<T> { | |
| return new Promise<T>((resolve, reject) => { | |
| const timer = setTimeout(() => { | |
| reject("Timeout"); | |
| }, this.timeoutMs); | |
| promise | |
| .then((value) => { | |
| clearTimeout(timer); | |
| resolve(value); | |
| }) | |
| .catch((error) => { | |
| clearTimeout(timer); | |
| reject(error); | |
| }); | |
| }); | |
| } | |
| } |
| * - throws → indicator reports `"down"` with the error message | ||
| * |
There was a problem hiding this comment.
The JSDoc says "throws → indicator reports "down" with the error message", but the implementation currently lets exceptions reject the returned promise (and the spec asserts a rejection). Either update the docs to reflect that check() may throw, or catch errors here and return a HealthIndicatorResult with status: 'down' and message.
| * - throws → indicator reports `"down"` with the error message | |
| * | |
| * | |
| * If the check function throws or returns a rejected promise, or if it does not | |
| * complete before the configured timeout, the returned promise from {@link IHealthIndicator.check} | |
| * will be rejected with the corresponding error. | |
| * |
| { | ||
| "name": "@ciscode/nestjs-developerkit", | ||
| "name": "@ciscode/health-kit", | ||
| "version": "1.0.0", | ||
| "description": "Template for NestJS developer kits (npm packages).", | ||
| "description": "NestJS health-check module — liveness & readiness probes with built-in PostgreSQL, Redis, and HTTP indicators.", | ||
| "author": "CisCode", |
There was a problem hiding this comment.
There is an existing changeset under .changeset/ that still targets the old package name (@ciscode/nestjs-developerkit). With the package rename to @ciscode/health-kit, changesets should be updated/added so versioning/changelog automation continues to work for the new package name.
| class SomeIndicator {} | ||
| class AnotherIndicator {} | ||
| class UndecotratedIndicator {} | ||
|
|
There was a problem hiding this comment.
Typo in class name: UndecotratedIndicator → UndecoratedIndicator. While test-only, this makes the code harder to read/search.
| * @example | ||
| * ```typescript | ||
| * HealthModule.register({ | ||
| * path: 'health', | ||
| * liveness: [], | ||
| * readiness: [ | ||
| * new HttpHealthIndicator('https://api.example.com/health'), | ||
| * ], | ||
| * }); | ||
| * ``` |
There was a problem hiding this comment.
The example refers to HealthModule.register(...), but the exported module is HealthKitModule. Update the example to use the correct module name so consumers can copy/paste it successfully.
| * @example | ||
| * ```typescript | ||
| * // With a pg.Pool | ||
| * const pool = new Pool({ connectionString: process.env.DATABASE_URL }); | ||
| * | ||
| * HealthModule.register({ | ||
| * path: 'health', | ||
| * liveness: [], | ||
| * readiness: [new PostgresHealthIndicator(pool)], | ||
| * }); | ||
| * ``` |
There was a problem hiding this comment.
The example refers to HealthModule.register(...), but this package exports HealthKitModule. Updating the example will prevent consumer confusion/copy-paste errors.
| * @example | ||
| * ```typescript | ||
| * import Redis from 'ioredis'; | ||
| * | ||
| * const redis = new Redis({ host: 'localhost', port: 6379 }); | ||
| * | ||
| * HealthModule.register({ | ||
| * path: 'health', | ||
| * liveness: [], | ||
| * readiness: [new RedisHealthIndicator(redis)], | ||
| * }); | ||
| * ``` |
There was a problem hiding this comment.
The example refers to HealthModule.register(...), but this package exports HealthKitModule. Update the snippet to the correct module name to keep docs accurate.
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes