diff --git a/src/app.ts b/src/app.ts index 84a00d4..9bfc3bc 100644 --- a/src/app.ts +++ b/src/app.ts @@ -15,10 +15,10 @@ export async function buildApp(): Promise { // Register Bootstrapped Plugins await bootstrapPlugins(app); - // Register Bootstrapped Global Routes - await bootstrapRoutes(app); - - // Global Error Handler + // Global Error Handler — MUST be registered before any route module is (bootstrapRoutes + // below), because Fastify resolves each encapsulated child context's error handler at the + // time that context is registered. A handler set after a child module has already been + // registered will not retroactively apply to that module's routes/hooks. app.setErrorHandler( (error: Error | AppError | ZodError, request: FastifyRequest, reply: FastifyReply) => { const requestId = request.reqContext?.requestId || (request.id as string) || 'unknown'; @@ -52,6 +52,25 @@ export async function buildApp(): Promise { }); } + // Framework-level errors (e.g. Fastify's own body-parser/payload errors) already carry + // their own client-facing statusCode/code — preserve those rather than reporting every + // non-AppError/ZodError as a 500, which would misclassify a client mistake as a server + // failure. + const frameworkStatusCode = (error as { statusCode?: number }).statusCode; + const frameworkCode = (error as { code?: string }).code; + if (frameworkStatusCode && frameworkStatusCode >= 400 && frameworkStatusCode < 500) { + logger.warn({ requestId, statusCode: frameworkStatusCode }, error.message); + return reply.status(frameworkStatusCode).send({ + success: false, + error: { + code: frameworkCode ?? 'BAD_REQUEST', + message: error.message, + details: null, + }, + requestId, + }); + } + logger.error({ error, requestId }, 'Unhandled Server Error'); const responseMessage = @@ -85,5 +104,9 @@ export async function buildApp(): Promise { }); }); + // Register Bootstrapped Global Routes — MUST come after the handlers above (see comment + // on setErrorHandler). + await bootstrapRoutes(app); + return app; } diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index d1d12f2..8a8541f 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -7,4 +7,32 @@ describe('App Factory Unit Test', () => { expect(app).toBeDefined(); await app.close(); }); + + it('applies the custom error envelope to errors raised inside a registered route module, not just root-level routes', async () => { + // Regression test: app.setErrorHandler() must be called BEFORE any route module is + // registered (bootstrapRoutes), because Fastify resolves each encapsulated child + // context's error handler at registration time — a handler set afterwards does not + // retroactively apply to already-registered child modules. This was discovered while + // building specs/002-saas-integration: every domain module's routes (registered via + // `app.register(someModuleRoutes)`, which creates a child encapsulation context) were + // silently falling back to Fastify's default `{statusCode, error, message}` shape + // instead of this app's `{success:false, error:{code,message,details}, requestId}` + // envelope. Uses malformed JSON against a real registered route (not a synthetic + // top-level route) so the error is raised by Fastify's own body parser inside that + // module's encapsulation context, with no database/Redis dependency — a regression + // here (e.g. someone moving setErrorHandler back after bootstrapRoutes) fails this test. + const app = await buildApp(); + const response = await app.inject({ + method: 'POST', + url: '/v1/support/requests', + headers: { 'content-type': 'application/json' }, + payload: 'not-valid-json{{{', + }); + + expect(response.statusCode).toBe(400); + const body = response.json(); + expect(body).toMatchObject({ success: false, error: { code: expect.any(String) } }); + expect(body.requestId).toBeDefined(); + await app.close(); + }); });