feat(fastify-core): harden lifecycle and readiness support

This commit is contained in:
saravanakumardb1 2026-03-06 12:47:29 -08:00
parent fb21c5d14d
commit acfad8a042
5 changed files with 485 additions and 44 deletions

View File

@ -22,8 +22,10 @@
10. [Implementation Backlog](#10-implementation-backlog)
11. [Success Metrics](#11-success-metrics)
12. [Recommended Rollout Order](#12-recommended-rollout-order)
13. [Appendix A: Audited Entry Points](#appendix-a-audited-entry-points)
14. [Appendix B: Proposed `fastify-core` API Evolution](#appendix-b-proposed-fastify-core-api-evolution)
13. [Validation and Rollout Guardrails](#13-validation-and-rollout-guardrails)
14. [Non-Goals](#14-non-goals)
15. [Appendix A: Audited Entry Points](#appendix-a-audited-entry-points)
16. [Appendix B: Proposed `fastify-core` API Evolution](#appendix-b-proposed-fastify-core-api-evolution)
---
@ -47,14 +49,15 @@ There are **no major request-path performance defects** identified in `@bytelyst
### Bottom Line
| Area | Assessment |
| --------------------------- | ------------------- |
| Wrapper adoption | Good |
| DRY at bootstrap layer | Partial |
| Consistency across services | Good but incomplete |
| Request-path performance | Acceptable |
| Operational maturity | Needs improvement |
| Risk of drift | Medium |
| Area | Assessment |
| ----------------------------------- | ------------------- |
| Wrapper adoption | Good |
| DRY at bootstrap layer | Partial |
| Consistency across services | Good but incomplete |
| Request-path performance | Acceptable |
| Operational maturity | Needs improvement |
| Test coverage for wrapper evolution | Incomplete |
| Risk of drift | Medium |
### Most Important Conclusions
@ -108,6 +111,12 @@ This audit focuses on:
This audit does **not** perform load testing, production benchmarking, or live traffic profiling.
### Scope Clarification
- **This is an audited-entrypoint review, not a proof of universal adoption in every historical file.**
- Conclusions in this document refer to the service entrypoints and shared package files listed in scope.
- If additional services or legacy bootstrap files exist outside the audited list, they should be checked separately before declaring workspace-wide completion.
---
## 3. Current State Snapshot
@ -187,6 +196,16 @@ Across repos, server entrypoints are highly recognizable. This improves onboardi
The wrapper does not contain an obvious throughput killer. Most of the remaining work is structural and operational, not about replacing Fastify or reworking request handling.
### 4.6 The current wrapper is intentionally small and therefore low-risk
The existing `fastify-core` package is still relatively narrow in scope. That is a strength because:
- it is easy to reason about
- it has limited hidden behavior
- service code remains explicit
This also means the next round of centralization should be done carefully to avoid turning the wrapper into an opaque framework.
---
## 5. Detailed Findings
@ -583,6 +602,105 @@ No urgent change required.
---
### F-014: Wrapper test coverage does not yet match the proposed roadmap breadth
| Field | Value |
| -------- | --------------------------- |
| Severity | Medium |
| Category | Testability / Change Safety |
| Status | Open |
**Description:**
The current `fastify-core` tests cover:
- basic app creation
- `/health`
- request ID propagation
- `ServiceError` handling
However, the roadmap proposes extending the wrapper with:
- readiness support
- auth hook helpers
- lifecycle changes in `startService(...)`
- optional plugin policy changes
- bootstrap orchestration
Those planned areas currently do not have equivalent shared-package test coverage in place.
**Impact:**
- Refactors may regress behavior across many repos at once
- Library-level changes will be harder to ship confidently
- Migration work may rely too heavily on manual verification
**Recommendation:**
Before or alongside each wrapper expansion:
- add focused tests in `packages/fastify-core`
- add at least one consumer integration test using a real service entrypoint
- verify shutdown and startup behavior without relying only on process-level side effects
---
### F-015: Centralizing too aggressively could turn the wrapper into an opaque internal framework
| Field | Value |
| -------- | ----------------- |
| Severity | Medium |
| Category | Architecture Risk |
| Status | Open |
**Description:**
The audit recommends centralizing more behavior into shared primitives. That is directionally correct, but there is a tradeoff: if too much service-specific logic gets hidden behind the wrapper, debugging and onboarding become harder.
**Risks:**
- entrypoints become less explicit
- product-specific boot order assumptions get buried in generic helpers
- future developers may need to understand a mini-framework before changing a service
**Recommendation:**
Adopt a strict rule for shared abstractions:
- `fastify-core` should own platform-wide mechanics
- service-specific business bootstrapping should remain visible in service code
- bootstrap helpers should orchestrate explicit hooks, not hide product behavior
---
### F-016: Migration compatibility and rollout risk is not yet called out strongly enough
| Field | Value |
| -------- | ------------ |
| Severity | Medium |
| Category | Rollout Risk |
| Status | Open |
**Description:**
Any change to `@bytelyst/fastify-core` affects multiple repositories and services. Even beneficial changes such as adding readiness, changing error response shape, or altering startup behavior can break:
- tests that snapshot response bodies
- operational scripts expecting current endpoints only
- local tooling relying on current exit behavior
- service entrypoints that assume certain startup sequencing
**Recommendation:**
Treat the wrapper as a cross-repo shared contract:
- roll out changes in additive mode first
- preserve backward-compatible defaults where possible
- migrate one reference consumer before broad rollout
- document breaking changes explicitly in the backlog
---
## 6. Duplication Inventory
### 6.1 Duplicated Patterns Observed
@ -612,6 +730,15 @@ No urgent change required.
The ecosystem has already centralized the **foundation**, but not yet the **repeated service policy layer**.
### 6.4 Duplication That Should Intentionally Remain
Not all repetition is bad. The following should remain explicit unless a concrete shared abstraction proves its value:
- service-specific route inventory
- service-specific startup tasks
- service-specific auth verifier configuration
- service-specific background jobs and caches
---
## 7. Performance and Operational Assessment
@ -706,6 +833,20 @@ Service startup should have explicit phases:
5. readiness transition
6. listen
### 8.5 Shared abstractions should be additive first, not mandatory first
The target architecture should prefer:
- new optional helpers with safe defaults
- migration by reference implementation
- preservation of existing entrypoint readability
It should avoid:
- one-shot rewrites across all repos
- hidden magic inside wrapper internals
- required breaking changes before migration support exists
---
## 9. Remediation Roadmap
@ -739,6 +880,7 @@ Service startup should have explicit phases:
- Add `requestId` to structured error responses
- Standardize optional plugin failure behavior
- Add readiness support (`/ready`)
- Add package-level tests for any new lifecycle/readiness behavior
### Priority
@ -760,6 +902,7 @@ The shared wrapper becomes safer, cleaner, and more suitable as a long-term plat
- Support verifier injection for product/service-specific token verification
- Standardize request decoration for `jwtPayload`
- Migrate all service entrypoints to shared auth helper
- Keep additive migration path so existing entrypoints can opt in incrementally
### Priority
@ -787,6 +930,7 @@ Auth parsing behavior becomes consistent across:
- Support pre-listen async tasks
- Support startup failure policy and readiness state
- Migrate services incrementally
- Keep bootstrap helper orchestration explicit rather than hiding product-specific init logic
### Priority
@ -839,22 +983,52 @@ The platform gets better observability with minimal risk.
---
## Phase 6 — Rollout Verification and Cleanup
**Goal:** Ensure all migration work is actually adopted and stable across consumers.
### Work Items
- Verify every audited service entrypoint still uses `@bytelyst/fastify-core`
- Remove superseded custom auth/bootstrap code after migration completes
- Update service templates and docs to reflect the new standard pattern
- Add regression tests or CI checks for wrapper adoption where practical
### Priority
**P1 / P2**
### Expected Outcome
The ecosystem finishes with fewer duplicate patterns and fewer partial migrations left behind.
---
## 10. Implementation Backlog
## Implementation Progress Log
- [ ] **Increment 1 — Phase 1 wrapper hardening**
- Scope: readiness support, requestId in error responses, configurable startup fatal-exit behavior, idempotent signal handling, optional plugin failure policy, wrapper tests
- Commit: `PENDING_COMMIT_SHA`
- Status: In progress
## P0 — Must Do
- [ ] Add `requestId` to all shared error responses
- [ ] Make `startService(...)` signal registration idempotent
- [ ] Stop forcing unconditional `process.exit(...)` from shared startup helper or make it configurable
- [x] Add `requestId` to all shared error responses
- [x] Make `startService(...)` signal registration idempotent
- [x] Stop forcing unconditional `process.exit(...)` from shared startup helper or make it configurable
- [ ] Create shared optional bearer JWT hook/helper
- [ ] Migrate all product backends and `mcp-server` to the shared auth helper
## P1 — Should Do
- [ ] Add `/ready` endpoint support
- [ ] Standardize optional plugin failure policy
- [x] Add `/ready` endpoint support
- [x] Standardize optional plugin failure policy
- [ ] Add structured bootstrap helper for init sequencing
- [ ] Migrate platform-service, extraction-service, and at least one product backend to the bootstrap helper
- [x] Add package-level tests covering lifecycle, readiness, and error response behavior
- [ ] Add integration tests covering wrapper behavior from at least one real service consumer
## P2 — Good Improvement
@ -862,12 +1036,14 @@ The platform gets better observability with minimal risk.
- [ ] Add startup timing diagnostics
- [ ] Add route/plugin summary in debug mode
- [ ] Improve typed request context model
- [ ] Add compatibility notes for any behavior changes in error shapes, readiness endpoints, or startup policy
## P3 — Nice to Have
- [ ] Add service template generator that assumes `bootstrapService(...)`
- [ ] Add lint or review checklist to discourage manual auth/bootstrap duplication
- [ ] Add CI checks for wrapper adoption in new service entrypoints
- [ ] Add a lightweight inventory script to report entrypoints not using shared helpers
---
@ -907,6 +1083,50 @@ Add shared bootstrap orchestration and migrate `platform-service` first, since i
Normalize route registration and operational diagnostics across the board.
### Step 6
Run a final adoption sweep and remove superseded custom bootstrap/auth code only after all consumers have migrated.
---
## 13. Validation and Rollout Guardrails
Every implementation phase should include both package-level and consumer-level verification.
### Package-Level Validation
- `pnpm --filter @bytelyst/fastify-core test`
- `pnpm --filter @bytelyst/fastify-core build`
### Service-Level Validation
- `pnpm --filter @lysnrai/platform-service test`
- `pnpm --filter @lysnrai/extraction-service test`
- `pnpm --filter @bytelyst/mcp-server test`
### Consumer Validation
At minimum, validate one representative product backend after each shared wrapper change.
### Rollout Guardrails
- make additive changes first
- avoid changing response shapes without documenting the contract change
- migrate one reference consumer before broad rollout
- remove duplicate legacy patterns only after successful adoption verification
---
## 14. Non-Goals
This roadmap does **not** currently aim to:
- replace Fastify with another backend framework
- collapse all service logic into one generic framework layer
- optimize for theoretical micro-benchmarks over maintainability
- force one-shot simultaneous migration across all repos
- hide product-specific startup logic that should remain explicit
---
## Appendix A: Audited Entry Points

View File

@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest';
import { createServiceApp } from '../index.js';
import { describe, expect, it, vi } from 'vitest';
import { createServiceApp, startService } from '../index.js';
describe('createServiceApp', () => {
it('returns a Fastify instance', async () => {
@ -90,6 +90,7 @@ describe('createServiceApp', () => {
expect(res.statusCode).toBe(404);
const body = JSON.parse(res.payload);
expect(body.error).toBe('User not found');
expect(body.requestId).toBeTruthy();
await app.close();
});
@ -111,6 +112,7 @@ describe('createServiceApp', () => {
const body = JSON.parse(res.payload);
expect(body.error).toBe('Validation failed');
expect(body.details).toEqual({ field: 'email' });
expect(body.requestId).toBeTruthy();
await app.close();
});
@ -130,6 +132,7 @@ describe('createServiceApp', () => {
expect(res.statusCode).toBe(500);
const body = JSON.parse(res.payload);
expect(body.error).toBe('Internal server error');
expect(body.requestId).toBeTruthy();
await app.close();
});
@ -147,4 +150,127 @@ describe('createServiceApp', () => {
await app.close();
});
it('supports readiness endpoint and returns not_ready until enabled', async () => {
const app = await createServiceApp({
name: 'ready-test',
version: '1.0.0',
readiness: true,
logger: false,
});
const before = await app.inject({ method: 'GET', url: '/ready' });
expect(before.statusCode).toBe(503);
expect(JSON.parse(before.payload)).toMatchObject({
status: 'not_ready',
service: 'ready-test',
});
app.setReadyState(true);
const after = await app.inject({ method: 'GET', url: '/ready' });
expect(after.statusCode).toBe(200);
expect(JSON.parse(after.payload)).toMatchObject({
status: 'ready',
service: 'ready-test',
});
await app.close();
});
it('supports custom readiness path and initial readiness state', async () => {
const app = await createServiceApp({
name: 'custom-ready',
version: '1.0.0',
readiness: { path: '/status/ready', initialReady: true },
logger: false,
});
const res = await app.inject({ method: 'GET', url: '/status/ready' });
expect(res.statusCode).toBe(200);
expect(JSON.parse(res.payload)).toMatchObject({
status: 'ready',
service: 'custom-ready',
});
await app.close();
});
it('can omit requestId from error responses when configured', async () => {
const { BadRequestError } = await import('@bytelyst/errors');
const app = await createServiceApp({
name: 'error-config',
version: '1.0.0',
logger: false,
errorResponses: { includeRequestId: false },
});
app.get('/bad', async () => {
throw new BadRequestError('No request id please');
});
const res = await app.inject({ method: 'GET', url: '/bad' });
expect(res.statusCode).toBe(400);
const body = JSON.parse(res.payload);
expect(body.error).toBe('No request id please');
expect(body.requestId).toBeUndefined();
await app.close();
});
});
describe('startService', () => {
it('sets readiness state after successful listen', async () => {
const app = await createServiceApp({
name: 'start-ready',
version: '1.0.0',
readiness: true,
logger: false,
});
const listenMock = vi.fn(async () => 'http://127.0.0.1:9999');
const closeMock = vi.fn(async () => undefined);
const infoMock = vi.fn();
const errorMock = vi.fn();
app.listen = listenMock as typeof app.listen;
app.close = closeMock as typeof app.close;
app.log.info = infoMock as typeof app.log.info;
app.log.error = errorMock as typeof app.log.error;
expect(app.isReadyState()).toBe(false);
await startService(app, { port: 9999, registerSignalHandlers: false, exitOnFatal: false });
expect(listenMock).toHaveBeenCalledWith({ port: 9999, host: '0.0.0.0' });
expect(app.isReadyState()).toBe(true);
expect(errorMock).not.toHaveBeenCalled();
await app.close();
});
it('throws startup error instead of exiting when exitOnFatal is false', async () => {
const app = await createServiceApp({
name: 'start-fail',
version: '1.0.0',
logger: false,
});
const startupError = new Error('listen failed');
const listenMock = vi.fn(async () => {
throw startupError;
});
const errorMock = vi.fn();
app.listen = listenMock as typeof app.listen;
app.log.error = errorMock as typeof app.log.error;
await expect(
startService(app, { port: 9999, registerSignalHandlers: false, exitOnFatal: false })
).rejects.toThrow('listen failed');
expect(errorMock).toHaveBeenCalledWith(startupError);
await app.close();
});
});

View File

@ -35,11 +35,24 @@ export async function createServiceApp(options: ServiceAppOptions): Promise<Fast
logLevel,
swagger,
metrics,
readiness,
errorResponses,
optionalPluginFailure = 'warn',
} = options;
const app = Fastify({
logger: logger ? (logLevel ? { level: logLevel } : true) : false,
});
}) as unknown as FastifyApp;
let readyState = typeof readiness === 'object' ? (readiness.initialReady ?? false) : false;
const readinessPath = typeof readiness === 'object' ? (readiness.path ?? '/ready') : '/ready';
const includeRequestIdInErrors = errorResponses?.includeRequestId ?? true;
app.setReadyState = (ready: boolean) => {
readyState = ready;
};
app.isReadyState = () => readyState;
// CORS — deny all origins when CORS_ORIGIN is not explicitly set
const origin = corsOrigin ? corsOrigin.split(',').map(o => o.trim()) : false;
@ -47,17 +60,27 @@ export async function createServiceApp(options: ServiceAppOptions): Promise<Fast
// OpenAPI spec (optional — consumer must have @fastify/swagger installed)
if (swagger) {
const swaggerPlugin = (await import('@fastify/swagger')).default;
await app.register(swaggerPlugin, {
openapi: {
info: {
title: swagger.title,
version,
...(swagger.description && { description: swagger.description }),
try {
const swaggerPlugin = (await import('@fastify/swagger')).default;
await app.register(swaggerPlugin, {
openapi: {
info: {
title: swagger.title,
version,
...(swagger.description && { description: swagger.description }),
},
...(swagger.port && { servers: [{ url: `http://localhost:${swagger.port}` }] }),
},
...(swagger.port && { servers: [{ url: `http://localhost:${swagger.port}` }] }),
},
});
});
} catch (error) {
if (optionalPluginFailure === 'throw') {
throw error;
}
app.log.warn(
{ err: error },
'Optional plugin @fastify/swagger not available; skipping OpenAPI registration'
);
}
// Swagger UI — serves interactive API docs at /documentation
try {
@ -66,16 +89,32 @@ export async function createServiceApp(options: ServiceAppOptions): Promise<Fast
routePrefix: '/documentation',
uiConfig: { docExpansion: 'list', deepLinking: true },
});
} catch {
// @fastify/swagger-ui not installed — skip silently
} catch (error) {
if (optionalPluginFailure === 'throw') {
throw error;
}
app.log.warn(
{ err: error },
'Optional plugin @fastify/swagger-ui not available; skipping Swagger UI registration'
);
}
}
// Prometheus metrics (optional — consumer must have fastify-metrics installed)
if (metrics) {
const metricsMod = await import('fastify-metrics');
const plugin = metricsMod.default as unknown as Parameters<typeof app.register>[0];
await app.register(plugin, { endpoint: '/metrics' });
try {
const metricsMod = await import('fastify-metrics');
const plugin = metricsMod.default as unknown as Parameters<typeof app.register>[0];
await app.register(plugin, { endpoint: '/metrics' });
} catch (error) {
if (optionalPluginFailure === 'throw') {
throw error;
}
app.log.warn(
{ err: error },
'Optional plugin fastify-metrics not available; skipping metrics registration'
);
}
}
// x-request-id propagation
@ -96,16 +135,36 @@ export async function createServiceApp(options: ServiceAppOptions): Promise<Fast
requestId: req.headers['x-request-id'],
}));
if (readiness) {
app.get(readinessPath, async (req, reply) => {
if (!app.isReadyState()) {
reply.code(503);
}
return {
status: app.isReadyState() ? 'ready' : 'not_ready',
service: name,
version,
...(description && { description }),
timestamp: new Date().toISOString(),
requestId: req.headers['x-request-id'],
};
});
}
// ServiceError-aware error handler
app.setErrorHandler((error, _req, reply) => {
app.setErrorHandler((error, req, reply) => {
if (error instanceof ServiceError) {
const body: Record<string, unknown> = { error: error.message };
if (error.details) body.details = error.details;
if (includeRequestIdInErrors) body.requestId = req.headers['x-request-id'];
reply.code(error.statusCode).send(body);
return;
}
app.log.error(error);
reply.code(500).send({ error: 'Internal server error' });
const body: Record<string, unknown> = { error: 'Internal server error' };
if (includeRequestIdInErrors) body.requestId = req.headers['x-request-id'];
reply.code(500).send(body);
});
return app;

View File

@ -4,23 +4,42 @@
import type { FastifyApp, StartOptions } from './types.js';
const registeredSignalHandlers = new Map<string, WeakSet<FastifyApp>>();
export async function startService(app: FastifyApp, options: StartOptions): Promise<void> {
const { port, host = '0.0.0.0' } = options;
const { port, host = '0.0.0.0', exitOnFatal = true, registerSignalHandlers = true } = options;
// Graceful shutdown on SIGTERM/SIGINT (Docker, K8s, Ctrl-C)
for (const signal of ['SIGTERM', 'SIGINT'] as const) {
process.on(signal, async () => {
app.log.info(`Received ${signal}, shutting down gracefully…`);
await app.close();
process.exit(0);
});
if (registerSignalHandlers) {
for (const signal of ['SIGTERM', 'SIGINT'] as const) {
const appsForSignal = registeredSignalHandlers.get(signal) ?? new WeakSet<FastifyApp>();
if (!appsForSignal.has(app)) {
appsForSignal.add(app);
registeredSignalHandlers.set(signal, appsForSignal);
process.once(signal, async () => {
app.log.info(`Received ${signal}, shutting down gracefully…`);
await app.close();
if (exitOnFatal) {
process.exit(0);
}
});
}
}
}
try {
await app.listen({ port, host });
if (typeof app.setReadyState === 'function') {
app.setReadyState(true);
}
app.log.info(`Service listening on ${host}:${port}`);
} catch (err) {
app.log.error(err);
process.exit(1);
if (exitOnFatal) {
process.exit(1);
}
throw err;
}
}

View File

@ -6,6 +6,15 @@ export interface SwaggerOptions {
port?: number;
}
export interface ReadinessOptions {
path?: string;
initialReady?: boolean;
}
export interface ErrorResponseOptions {
includeRequestId?: boolean;
}
export interface ServiceAppOptions {
name: string;
version: string;
@ -15,11 +24,19 @@ export interface ServiceAppOptions {
logLevel?: 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'fatal';
swagger?: SwaggerOptions;
metrics?: boolean;
readiness?: boolean | ReadinessOptions;
errorResponses?: ErrorResponseOptions;
optionalPluginFailure?: 'warn' | 'throw';
}
export interface StartOptions {
port: number;
host?: string;
exitOnFatal?: boolean;
registerSignalHandlers?: boolean;
}
export type FastifyApp = FastifyInstance;
export type FastifyApp = FastifyInstance & {
setReadyState: (ready: boolean) => void;
isReadyState: () => boolean;
};