feat: add syncing_status query parameter to eth/v1/node/health

packages/api/src/beacon/routes/node.ts

@@ -70,6 +70,10 @@ export enum NodeHealth {
   NOT_INITIALIZED_OR_ISSUES = HttpStatusCode.SERVICE_UNAVAILABLE,
 }
 
+export type NodeHealthOptions = {
+  syncingStatus?: number;
+};
+
 /**
  * Read information about the beacon node.
  */
@@ -120,13 +124,13 @@ export type Api = {
   /**
    * Get health check
    * Returns node health status in http status codes. Useful for load balancers.
-   *
-   * NOTE: This route does not return any value
    */
-  getHealth(): Promise<
+  getHealth(
+    options?: NodeHealthOptions
+  ): Promise<
     ApiClientResponse<
       {[HttpStatusCode.OK]: void; [HttpStatusCode.PARTIAL_CONTENT]: void},
-      HttpStatusCode.SERVICE_UNAVAILABLE
+      HttpStatusCode.BAD_REQUEST | HttpStatusCode.SERVICE_UNAVAILABLE
     >
   >;
 };
@@ -150,7 +154,7 @@ export type ReqTypes = {
   getPeerCount: ReqEmpty;
   getNodeVersion: ReqEmpty;
   getSyncingStatus: ReqEmpty;
-  getHealth: ReqEmpty;
+  getHealth: {query: {syncing_status?: number}};
 };
 
 export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
@@ -171,7 +175,13 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
     getPeerCount: reqEmpty,
     getNodeVersion: reqEmpty,
     getSyncingStatus: reqEmpty,
-    getHealth: reqEmpty,
+    getHealth: {
+      writeReq: (options) => ({
+        query: options?.syncingStatus !== undefined ? {syncing_status: options.syncingStatus} : {},
+      }),
+      parseReq: ({query}) => [{syncingStatus: query.syncing_status}],
+      schema: {query: {syncing_status: Schema.Uint}},
+    },
   };
 }
 

packages/api/src/beacon/server/node.ts

@@ -1,32 +1,9 @@
 import {ChainForkConfig} from "@lodestar/config";
 import {Api, ReqTypes, routesData, getReturnTypes, getReqSerializers} from "../routes/node.js";
 import {ServerRoutes, getGenericJsonServer} from "../../utils/server/index.js";
-import {HttpStatusCode} from "../../utils/client/httpStatusCode.js";
 import {ServerApi} from "../../interfaces.js";
 
 export function getRoutes(config: ChainForkConfig, api: ServerApi<Api>): ServerRoutes<ServerApi<Api>, ReqTypes> {
   // All routes return JSON, use a server auto-generator
-  const serverRoutes = getGenericJsonServer<ServerApi<Api>, ReqTypes>(
-    {routesData, getReturnTypes, getReqSerializers},
-    config,
-    api
-  );
-
-  return {
-    ...serverRoutes,
-
-    getHealth: {
-      ...serverRoutes.getHealth,
-      handler: async (req, res) => {
-        try {
-          await api.getHealth();
-          res.raw.writeHead(HttpStatusCode.OK);
-          res.raw.end();
-        } catch (e) {
-          res.raw.writeHead(HttpStatusCode.INTERNAL_SERVER_ERROR);
-          res.raw.end();
-        }
-      },
-    },
-  };
+  return getGenericJsonServer<ServerApi<Api>, ReqTypes>({routesData, getReturnTypes, getReqSerializers}, config, api);
 }

packages/api/src/interfaces.ts

@@ -7,7 +7,7 @@ export type APIClientHandler = (...args: any) => PromiseLike<ApiClientResponse>;
 export type APIServerHandler = (...args: any) => PromiseLike<unknown>;
 
 export type ApiClientSuccessResponse<S extends keyof any, T> = {ok: true; status: S; response: T; error?: never};
-export type ApiClientSErrorResponse<S extends Exclude<HttpStatusCode, HttpSuccessCodes>> = {
+export type ApiClientErrorResponse<S extends Exclude<HttpStatusCode, HttpSuccessCodes>> = {
   ok: false;
   status: S;
   response?: never;
@@ -18,8 +18,8 @@ export type ApiClientResponse<
   E extends Exclude<HttpStatusCode, HttpSuccessCodes> = Exclude<HttpStatusCode, HttpSuccessCodes>,
 > =
   | {[K in keyof S]: ApiClientSuccessResponse<K, S[K]>}[keyof S]
-  | {[K in E]: ApiClientSErrorResponse<K>}[E]
-  | ApiClientSErrorResponse<HttpStatusCode.INTERNAL_SERVER_ERROR>;
+  | {[K in E]: ApiClientErrorResponse<K>}[E]
+  | ApiClientErrorResponse<HttpStatusCode.INTERNAL_SERVER_ERROR>;
 
 export type ApiClientResponseData<T extends ApiClientResponse> = T extends {ok: true; response: infer R} ? R : never;
 

packages/api/src/utils/client/client.ts

@@ -26,7 +26,7 @@ export function getFetchOptsSerializer<Fn extends (...args: any) => any, ReqType
     return {
       url: urlFormater(req.params ?? {}),
       method: routeDef.method,
-      query: req.query,
+      query: Object.keys(req.query ?? {}).length ? req.query : undefined,
       body: req.body as unknown,
       headers: req.headers,
       routeId,
@@ -80,14 +80,17 @@ export function generateGenericJsonClient<
         }
       } catch (err) {
         if (err instanceof HttpError) {
-          return {ok: false, error: {code: err.status, message: err.message, operationId: routeId}} as ReturnType<
-            Api[keyof Api]
-          >;
+          return {
+            ok: false,
+            status: err.status,
+            error: {code: err.status, message: err.message, operationId: routeId},
+          } as ReturnType<Api[keyof Api]>;
         }
 
         if (err instanceof TimeoutError) {
           return {
             ok: false,
+            status: HttpStatusCode.INTERNAL_SERVER_ERROR,
             error: {code: HttpStatusCode.INTERNAL_SERVER_ERROR, message: err.message, operationId: routeId},
           } as ReturnType<Api[keyof Api]>;
         }

packages/api/src/utils/server/genericJsonServer.ts

@@ -42,7 +42,7 @@ export function getGenericJsonServer<
 
       handler: async function handler(this: FastifyInstance, req, resp): Promise<unknown | void> {
         const args: any[] = routeSerdes.parseReq(req as ReqGeneric as ReqTypes[keyof Api]);
-        const data = (await api[routeId](...args)) as Resolves<Api[keyof Api]>;
+        const data = (await api[routeId](...args, req, resp)) as Resolves<Api[keyof Api]>;
 
         if (routeDef.statusOk !== undefined) {
           resp.statusCode = routeDef.statusOk;

packages/api/test/unit/beacon/testData/node.ts

@@ -52,7 +52,7 @@ export const testData: GenericServerTestCases<Api> = {
     res: {data: {headSlot: "1", syncDistance: "2", isSyncing: false, isOptimistic: true, elOffline: false}},
   },
   getHealth: {
-    args: [],
+    args: [{syncingStatus: 206}],
     res: undefined,
   },
 };

packages/api/test/utils/genericServerTest.ts

@@ -1,3 +1,5 @@
+import {ServerResponse} from "node:http";
+import {FastifyReply} from "fastify";
 import {expect} from "chai";
 import {ChainForkConfig} from "@lodestar/config";
 import {ReqGeneric, Resolves} from "../../src/utils/index.js";
@@ -58,7 +60,15 @@ export function runGenericServerTest<
 
       // Assert server handler called with correct args
       expect(mockApi[routeId].callCount).to.equal(1, `mockApi[${routeId as string}] must be called once`);
-      expect(mockApi[routeId].getCall(0).args).to.deep.equal(testCase.args, `mockApi[${routeId as string}] wrong args`);
+      let {args} = mockApi[routeId].getCall(0);
+      // Default handler injects request and response as last two args
+      const lastArg = args[args.length - 1];
+      // Detect if last arg is response as custom handlers might not inject it
+      if (typeof lastArg === "object" && (lastArg as FastifyReply).raw instanceof ServerResponse) {
+        // Remove request and response before assertion
+        args = args.slice(0, -2) as typeof args;
+      }
+      expect(args).to.deep.equal(testCase.args, `mockApi[${routeId as string}] wrong args`);
 
       // Assert returned value is correct
       expect(res.response).to.deep.equal(testCase.res, "Wrong returned value");

packages/beacon-node/src/api/impl/node/index.ts

@@ -66,10 +66,18 @@ export function getNodeApi(
       return {data: sync.getSyncStatus()};
     },
 
-    async getHealth(_req, res) {
+    async getHealth(options, _req, res) {
+      // Custom value passed via `syncing_status` query parameter
+      const syncingStatus = options?.syncingStatus;
+
+      // Syncing status must be a valid HTTP status code within range 100-599
+      if (syncingStatus != null && (syncingStatus < 100 || syncingStatus > 599)) {
+        throw new ApiError(400, `Invalid syncing status code: ${syncingStatus}`);
+      }
+
       if (sync.getSyncStatus().isSyncing) {
         // 206: Node is syncing but can serve incomplete data
-        res?.code(routes.node.NodeHealth.SYNCING);
+        res?.code(syncingStatus ?? routes.node.NodeHealth.SYNCING);
       } else {
         // 200: Node is ready
         res?.code(routes.node.NodeHealth.READY);

packages/beacon-node/test/e2e/api/impl/beacon/node/endpoints.test.ts

@@ -34,7 +34,7 @@ describe("beacon node api", function () {
         chain: {blsVerifyAllMainThread: true},
       },
       validatorCount,
-      logger: testLogger("Node-A", {level: LogLevel.info}),
+      logger: testLogger("Node-Synced", {level: LogLevel.info}),
     });
     client = getClient({baseUrl: `http://127.0.0.1:${restPort}`}, {config});
   });
@@ -65,9 +65,8 @@ describe("beacon node api", function () {
     });
 
     it("should return 'el_offline' as 'true' when EL not available", async () => {
-      // Close first instance
-      await bn.close();
-      bn = await getDevBeaconNode({
+      const portElOffline = 9597;
+      const bnElOffline = await getDevBeaconNode({
         params: {
           ...chainConfigDef,
           // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -82,18 +81,19 @@ describe("beacon node api", function () {
           api: {
             rest: {
               enabled: true,
-              port: restPort,
+              port: portElOffline,
             },
           },
           chain: {blsVerifyAllMainThread: true},
         },
         validatorCount: 5,
-        logger: testLogger("Node-A", {level: LogLevel.info}),
+        logger: testLogger("Node-EL-Offline", {level: LogLevel.info}),
       });
+      const clientElOffline = getClient({baseUrl: `http://127.0.0.1:${portElOffline}`}, {config});
 
       // To make BN communicate with EL, it needs to produce some blocks and for that need validators
       const {validators} = await getAndInitDevValidators({
-        node: bn,
+        node: bnElOffline,
         validatorClientCount: 1,
         validatorsPerClient: validatorCount,
         startIndex: 0,
@@ -102,12 +102,75 @@ describe("beacon node api", function () {
       // Give node sometime to communicate with EL
       await sleep(chainConfigDef.SECONDS_PER_SLOT * 2 * 1000);
 
-      const res = await client.node.getSyncingStatus();
+      const res = await clientElOffline.node.getSyncingStatus();
       ApiError.assert(res);
 
       expect(res.response.data.elOffline).to.eql(true);
 
       await Promise.all(validators.map((v) => v.close()));
+      await bnElOffline.close();
+    });
+  });
+
+  describe("getHealth", () => {
+    const portSyncing = 9598;
+
+    let bnSyncing: BeaconNode;
+    let clientSyncing: Api;
+
+    before(async () => {
+      bnSyncing = await getDevBeaconNode({
+        params: chainConfigDef,
+        options: {
+          // Node won't consider itself synced without peers
+          sync: {isSingleNode: false},
+          api: {
+            rest: {
+              enabled: true,
+              port: portSyncing,
+            },
+          },
+          chain: {blsVerifyAllMainThread: true},
+        },
+        validatorCount,
+        logger: testLogger("Node-Syncing", {level: LogLevel.info}),
+      });
+      clientSyncing = getClient({baseUrl: `http://127.0.0.1:${portSyncing}`}, {config});
+      // Must at least wait for one slot else node considers itself synced pre/at genesis
+      await sleep(chainConfigDef.SECONDS_PER_SLOT * 1000);
+    });
+
+    after(async () => {
+      await bnSyncing.close();
+    });
+
+    it("should return 200 status code if node is ready", async () => {
+      const res = await client.node.getHealth();
+      expect(res.status).to.equal(200);
+    });
+
+    it("should return 206 status code if node is syncing", async () => {
+      const res = await clientSyncing.node.getHealth();
+      expect(res.status).to.equal(206);
+    });
+
+    it("should return custom status code from 'syncing_status' query parameter if node is syncing", async () => {
+      const statusCode = 204;
+      const res = await clientSyncing.node.getHealth({syncingStatus: statusCode});
+      expect(res.status).to.equal(statusCode);
+    });
+
+    it("should only use status code from 'syncing_status' query parameter if node is syncing", async () => {
+      const res = await client.node.getHealth({syncingStatus: 204});
+      expect(res.status).to.equal(200);
+    });
+
+    it("should return 400 status code if value of 'syncing_status' query parameter is invalid", async () => {
+      const res = await clientSyncing.node.getHealth({syncingStatus: 99});
+      expect(res.status).to.equal(400);
+
+      const resp = await clientSyncing.node.getHealth({syncingStatus: 600});
+      expect(resp.status).to.equal(400);
     });
   });
 });