diff --git a/packages/api/src/beacon/routes/node.ts b/packages/api/src/beacon/routes/node.ts index 9f1aa8b3cc09..9a954fe6ad57 100644 --- a/packages/api/src/beacon/routes/node.ts +++ b/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 { @@ -171,7 +175,13 @@ export function getReqSerializers(): ReqSerializers { 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}}, + }, }; } diff --git a/packages/api/src/beacon/server/node.ts b/packages/api/src/beacon/server/node.ts index c96f4ad39be3..8ee3acdb426b 100644 --- a/packages/api/src/beacon/server/node.ts +++ b/packages/api/src/beacon/server/node.ts @@ -1,11 +1,11 @@ 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): ServerRoutes, ReqTypes> { - // All routes return JSON, use a server auto-generator + const reqSerializers = getReqSerializers(); + const serverRoutes = getGenericJsonServer, ReqTypes>( {routesData, getReturnTypes, getReqSerializers}, config, @@ -18,14 +18,11 @@ export function getRoutes(config: ChainForkConfig, api: ServerApi): ServerR 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(); - } + const args = reqSerializers.getHealth.parseReq(req); + // Note: This type casting is required as per route definition getHealth + // does not return a value but since the internal API does not have access + // to response object it is required to set the HTTP status code here. + res.statusCode = (await api.getHealth(...args)) as unknown as number; }, }, }; diff --git a/packages/api/src/interfaces.ts b/packages/api/src/interfaces.ts index b31d6ddfc10e..166596297ace 100644 --- a/packages/api/src/interfaces.ts +++ b/packages/api/src/interfaces.ts @@ -7,7 +7,7 @@ export type APIClientHandler = (...args: any) => PromiseLike; export type APIServerHandler = (...args: any) => PromiseLike; export type ApiClientSuccessResponse = {ok: true; status: S; response: T; error?: never}; -export type ApiClientSErrorResponse> = { +export type ApiClientErrorResponse> = { ok: false; status: S; response?: never; @@ -18,17 +18,16 @@ export type ApiClientResponse< E extends Exclude = Exclude, > = | {[K in keyof S]: ApiClientSuccessResponse}[keyof S] - | {[K in E]: ApiClientSErrorResponse}[E] - | ApiClientSErrorResponse; + | {[K in E]: ApiClientErrorResponse}[E] + | ApiClientErrorResponse; export type ApiClientResponseData = T extends {ok: true; response: infer R} ? R : never; -export type GenericRequestObject = Record; -export type GenericResponseObject = {code: (code: number) => void}; +export type GenericOptions = Record; export type ServerApi> = { [K in keyof T]: ( - ...args: [...args: Parameters, req?: GenericRequestObject, res?: GenericResponseObject] + ...args: [...args: Parameters, opts?: GenericOptions] ) => Promise>>; }; diff --git a/packages/api/src/utils/client/client.ts b/packages/api/src/utils/client/client.ts index 55b5e20f628e..a88b68553898 100644 --- a/packages/api/src/utils/client/client.ts +++ b/packages/api/src/utils/client/client.ts @@ -26,7 +26,7 @@ export function getFetchOptsSerializer 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; } 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; } diff --git a/packages/api/test/unit/beacon/testData/node.ts b/packages/api/test/unit/beacon/testData/node.ts index 1c1b835edeec..2243f37fc4c7 100644 --- a/packages/api/test/unit/beacon/testData/node.ts +++ b/packages/api/test/unit/beacon/testData/node.ts @@ -52,7 +52,7 @@ export const testData: GenericServerTestCases = { res: {data: {headSlot: "1", syncDistance: "2", isSyncing: false, isOptimistic: true, elOffline: false}}, }, getHealth: { - args: [], + args: [{syncingStatus: 206}], res: undefined, }, }; diff --git a/packages/beacon-node/src/api/impl/node/index.ts b/packages/beacon-node/src/api/impl/node/index.ts index 1ea3e6b37665..26b7f827d1fd 100644 --- a/packages/beacon-node/src/api/impl/node/index.ts +++ b/packages/beacon-node/src/api/impl/node/index.ts @@ -66,18 +66,29 @@ export function getNodeApi( return {data: sync.getSyncStatus()}; }, - async getHealth(_req, res) { + async getHealth(options) { + const syncingStatus = options?.syncingStatus; + + if (syncingStatus != null && (syncingStatus < 100 || syncingStatus > 599)) { + throw new ApiError(400, `Invalid syncing status code: ${syncingStatus}`); + } + + let healthStatus: number; + if (sync.getSyncStatus().isSyncing) { // 206: Node is syncing but can serve incomplete data - res?.code(routes.node.NodeHealth.SYNCING); + healthStatus = syncingStatus ?? routes.node.NodeHealth.SYNCING; } else { // 200: Node is ready - res?.code(routes.node.NodeHealth.READY); + healthStatus = routes.node.NodeHealth.READY; } // else { // 503: Node not initialized or having issues // NOTE: Lodestar does not start its API until fully initialized, so this status can never be served // } + + // Health status is returned to allow route handler to set it as HTTP status code + return healthStatus as unknown as void; }, }; } diff --git a/packages/beacon-node/test/e2e/api/impl/beacon/node/endpoints.test.ts b/packages/beacon-node/test/e2e/api/impl/beacon/node/endpoints.test.ts index ae19523b8a03..a9f672c25d62 100644 --- a/packages/beacon-node/test/e2e/api/impl/beacon/node/endpoints.test.ts +++ b/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); }); }); });