Skip to content

Commit

Permalink
Add syncing_status query parameter to eth/v1/node/health
Browse files Browse the repository at this point in the history
  • Loading branch information
nflaig committed Jul 23, 2023
1 parent 3257345 commit 5f25f03
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 43 deletions.
18 changes: 13 additions & 5 deletions packages/api/src/beacon/routes/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -120,10 +124,10 @@ 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
Expand All @@ -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> {
Expand All @@ -171,7 +175,11 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
getPeerCount: reqEmpty,
getNodeVersion: reqEmpty,
getSyncingStatus: reqEmpty,
getHealth: reqEmpty,
getHealth: {
writeReq: (options) => ({query: {syncing_status: options?.syncingStatus}}),
parseReq: ({query}) => [{syncingStatus: query.syncing_status}],
schema: {query: {syncing_status: Schema.Uint}},
},
};
}

Expand Down
25 changes: 1 addition & 24 deletions packages/api/src/beacon/server/node.ts
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion packages/api/src/utils/server/genericJsonServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/api/test/unit/beacon/testData/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
12 changes: 11 additions & 1 deletion packages/api/test/utils/genericServerTest.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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");
Expand Down
16 changes: 13 additions & 3 deletions packages/beacon-node/src/api/impl/node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,20 @@ export function getNodeApi(
return {data: sync.getSyncStatus()};
},

async getHealth(_req, res) {
async getHealth(options, _req, res) {
if (sync.getSyncStatus().isSyncing) {
// 206: Node is syncing but can serve incomplete data
res?.code(routes.node.NodeHealth.SYNCING);
if (options?.syncingStatus != null) {
// Custom value passed via `syncing_status` query parameter
const {syncingStatus} = options;
// Must be a valid HTTP status code within range 100-599
if (syncingStatus < 100 || syncingStatus > 599) {
throw new ApiError(400, `Invalid syncing status code: ${syncingStatus}`);
}
res?.code(syncingStatus);
} else {
// 206: Node is syncing but can serve incomplete data
res?.code(routes.node.NodeHealth.SYNCING);
}
} else {
// 200: Node is ready
res?.code(routes.node.NodeHealth.READY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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});
});
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -102,12 +102,74 @@ 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);

// Status code should only be customizable if node is syncing
const resp = await client.node.getHealth({syncingStatus: 204});
expect(resp.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 return 400 status code if value in 'syncing_status' query parameter is invalid", async () => {
const res = await clientSyncing.node.getHealth({syncingStatus: 1});
expect(res.error?.code).to.equal(400);

const resp = await clientSyncing.node.getHealth({syncingStatus: 600});
expect(resp.error?.code).to.equal(400);
});
});
});

0 comments on commit 5f25f03

Please sign in to comment.