Clarify health endpoint route conflicts

Author: justin808

packages/react-on-rails-pro-node-renderer/src/worker.ts

@@ -30,7 +30,7 @@ import packageJson from './shared/packageJson.js';
 import { buildConfig, Config, getConfig } from './shared/configBuilder.js';
 import fileExistsAsync from './shared/fileExistsAsync.js';
 import { runRscPeerCompatibilityCheck } from './shared/runRscPeerCompatibilityCheck.js';
-import type { FastifyReply } from './worker/types.js';
+import type { FastifyInstance, FastifyReply } from './worker/types.js';
 import { performRequestPrechecks } from './worker/requestPrechecks.js';
 import { type AuthBody, authenticate } from './worker/authHandler.js';
 import {
@@ -84,6 +84,9 @@ declare module 'fastify' {
   }
 }
 
+const HEALTH_ENDPOINT_ROUTES = ['/health', '/ready'] as const;
+const FASTIFY_DUPLICATED_ROUTE_ERROR_CODE = 'FST_ERR_DUPLICATED_ROUTE';
+
 function setHeaders(headers: ResponseResult['headers'], res: FastifyReply) {
   // eslint-disable-next-line @typescript-eslint/no-misused-promises -- fixing it with `void` just violates no-void
   Object.entries(headers).forEach(([key, header]) => res.header(key, header));
@@ -106,6 +109,42 @@ const setResponse = async (result: ResponseResult, res: FastifyReply) => {
 
 const isAsset = (value: unknown): value is Asset => (value as { type?: string }).type === 'asset';
 
+function conflictingHealthEndpointPath(error: unknown): (typeof HEALTH_ENDPOINT_ROUTES)[number] | undefined {
+  if (typeof error !== 'object' || error === null) {
+    return undefined;
+  }
+
+  const { code, message } = error as { code?: unknown; message?: unknown };
+  if (code !== FASTIFY_DUPLICATED_ROUTE_ERROR_CODE || typeof message !== 'string') {
+    return undefined;
+  }
+
+  return HEALTH_ENDPOINT_ROUTES.find((routePath) => message.includes(`route '${routePath}'`));
+}
+
+function applyFastifyConfigWithHealthEndpointMigrationHint(
+  app: FastifyInstance,
+  enableHealthEndpoints: boolean,
+) {
+  try {
+    applyFastifyConfigFunctions(app);
+  } catch (error) {
+    const conflictingPath = enableHealthEndpoints ? conflictingHealthEndpointPath(error) : undefined;
+    if (conflictingPath) {
+      const originalMessage = error instanceof Error ? error.message : String(error);
+      const message =
+        `enableHealthEndpoints registers built-in GET ${conflictingPath}, but a configureFastify callback ` +
+        `already registered that route. Remove or rename the custom ${conflictingPath} route when migrating ` +
+        'to the built-in health endpoints. See docs/oss/building-features/node-renderer/health-checks.md.';
+
+      log.error({ err: error, route: conflictingPath }, message);
+      throw new Error(`${message} Original Fastify error: ${originalMessage}`);
+    }
+
+    throw error;
+  }
+}
+
 function assertAsset(value: unknown, key: string): asserts value is Asset {
   if (!isAsset(value)) {
     throw new Error(`React On Rails Error: Expected an asset for key: ${key}`);
@@ -681,7 +720,7 @@ export default function run(config: Partial<Config>) {
       if (hasAnyVMContext()) {
         res.send({ status: 'ready' });
       } else {
-        res.status(503).send({ status: 'waiting_for_bundle' });
+        res.status(503).header('Retry-After', '5').send({ status: 'waiting_for_bundle' });
       }
     });
   }
@@ -703,7 +742,7 @@ export default function run(config: Partial<Config>) {
 
   // Integration hooks registered before the worker loads are applied here, immediately after
   // listen() is scheduled and before Fastify finishes booting.
-  applyFastifyConfigFunctions(app);
+  applyFastifyConfigWithHealthEndpointMigrationHint(app, enableHealthEndpoints);
 
   return app;
 }

packages/react-on-rails-pro-node-renderer/tests/healthEndpoints.test.ts

@@ -17,7 +17,8 @@ import formAutoContent from 'form-auto-content';
 import { createReadStream } from 'fs-extra';
 // eslint-disable-next-line import/no-relative-packages
 import packageJson from '../package.json';
-import worker, { disableHttp2 } from '../src/worker';
+import worker, { configureFastify, disableHttp2 } from '../src/worker';
+import { __resetFastifyConfigFunctionsForTest } from '../src/worker/fastifyConfig';
 import { BUNDLE_TIMESTAMP, getFixtureBundle, resetForTest, serverBundleCachePath } from './helper';
 
 const testName = 'healthEndpoints';
@@ -60,12 +61,14 @@ describe('built-in health endpoints', () => {
 
   beforeEach(async () => {
     delete process.env.RENDERER_ENABLE_HEALTH_ENDPOINTS;
+    __resetFastifyConfigFunctionsForTest();
     await resetForTest(testName);
   });
 
   afterEach(async () => {
     await app?.close();
     app = undefined;
+    __resetFastifyConfigFunctionsForTest();
   });
 
   afterAll(async () => {
@@ -107,6 +110,7 @@ describe('built-in health endpoints', () => {
 
     const readyRes = await app.inject().get('/ready').end();
     expect(readyRes.statusCode).toBe(503);
+    expect(readyRes.headers['retry-after']).toBe('5');
     expect(JSON.parse(readyRes.payload)).toEqual({ status: 'waiting_for_bundle' });
   });
 
@@ -116,6 +120,7 @@ describe('built-in health endpoints', () => {
     // No bundle compiled into the VM pool yet: not ready.
     const notReadyRes = await app.inject().get('/ready').end();
     expect(notReadyRes.statusCode).toBe(503);
+    expect(notReadyRes.headers['retry-after']).toBe('5');
     expect(JSON.parse(notReadyRes.payload)).toEqual({ status: 'waiting_for_bundle' });
 
     // First render request uploads and compiles the bundle.
@@ -141,4 +146,16 @@ describe('built-in health endpoints', () => {
     expect(healthAfter.statusCode).toBe(200);
     expect(JSON.parse(healthAfter.payload)).toEqual({ status: 'ok' });
   });
+
+  test('reports a migration hint when a custom health route conflicts with built-in endpoints', () => {
+    configureFastify((fastifyApp) => {
+      fastifyApp.get('/health', (_req, res) => {
+        res.send({ status: 'legacy' });
+      });
+    });
+
+    expect(() => createWorker({ enableHealthEndpoints: true })).toThrow(
+      /enableHealthEndpoints registers built-in GET \/health/,
+    );
+  });
 });