From 1aeebe1ff1b56ad5ca32d0a351581dcd47b2c251 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 26 Sep 2024 15:39:36 +0100 Subject: [PATCH 1/9] fix: update HTTP error response format to be spec compliant --- packages/beacon-node/src/api/rest/base.ts | 38 +++++++++++++++++-- packages/beacon-node/src/api/rest/index.ts | 1 + packages/cli/src/cmds/dev/options.ts | 4 ++ packages/cli/src/cmds/validator/handler.ts | 1 + .../src/cmds/validator/keymanager/server.ts | 1 + packages/cli/src/cmds/validator/options.ts | 7 ++++ .../cli/src/options/beaconNodeOptions/api.ts | 9 +++++ packages/cli/test/sim/endpoints.test.ts | 16 +++++++- .../unit/options/beaconNodeOptions.test.ts | 2 + 9 files changed, 75 insertions(+), 4 deletions(-) diff --git a/packages/beacon-node/src/api/rest/base.ts b/packages/beacon-node/src/api/rest/base.ts index 5f191bf76beb..e8ea85f67af5 100644 --- a/packages/beacon-node/src/api/rest/base.ts +++ b/packages/beacon-node/src/api/rest/base.ts @@ -15,6 +15,7 @@ export type RestApiServerOpts = { bearerToken?: string; headerLimit?: number; bodyLimit?: number; + stacktraces?: boolean; swaggerUI?: boolean; }; @@ -29,11 +30,27 @@ export type RestApiServerMetrics = SocketMetrics & { errors: Gauge<{operationId: string}>; }; +/** + * Error response body format as defined in beacon-api spec + * + * See https://github.com/ethereum/beacon-APIs/blob/v2.5.0/types/http.yaml + */ +type ErrorResponse = { + code: number; + message: string; + stacktraces?: string[]; +}; + /** * Error code used by Fastify if media type is not supported (415) */ const INVALID_MEDIA_TYPE_CODE = errorCodes.FST_ERR_CTP_INVALID_MEDIA_TYPE().code; +/** + * Error code used by Fastify if JSON schema validation failed + */ +const SCHEMA_VALIDATION_ERROR_CODE = errorCodes.FST_ERR_VALIDATION().code; + /** * REST API powered by `fastify` server. */ @@ -71,15 +88,30 @@ export class RestApiServer { // To parse our ApiError -> statusCode server.setErrorHandler((err, _req, res) => { + const stacktraces = opts.stacktraces ? err.stack?.split("\n") : undefined; if (err.validation) { - void res.status(400).send(err.validation); + const {instancePath, message} = err.validation[0]; + const payload: ErrorResponse = { + code: err.statusCode ?? 400, + message: `${instancePath.substring(instancePath.lastIndexOf("/") + 1)} ${message}`, + stacktraces, + }; + void res.status(400).send(payload); } else { // Convert our custom ApiError into status code const statusCode = err instanceof ApiError ? err.statusCode : 500; - void res.status(statusCode).send(err); + const payload: ErrorResponse = {code: statusCode, message: err.message, stacktraces}; + void res.status(statusCode).send(payload); } }); + server.setNotFoundHandler((req, res) => { + const message = `Route ${req.raw.method}:${req.raw.url} not found`; + this.logger.warn(message); + const payload: ErrorResponse = {code: 404, message}; + void res.code(404).send(payload); + }); + if (opts.cors) { void server.register(fastifyCors, {origin: opts.cors}); } @@ -127,7 +159,7 @@ export class RestApiServer { const operationId = getOperationId(req); - if (err instanceof ApiError || err.code === INVALID_MEDIA_TYPE_CODE) { + if (err instanceof ApiError || [INVALID_MEDIA_TYPE_CODE, SCHEMA_VALIDATION_ERROR_CODE].includes(err.code)) { this.logger.warn(`Req ${req.id} ${operationId} failed`, {reason: err.message}); } else { this.logger.error(`Req ${req.id} ${operationId} error`, {}, err); diff --git a/packages/beacon-node/src/api/rest/index.ts b/packages/beacon-node/src/api/rest/index.ts index e27ed6bd9139..6beaf061588c 100644 --- a/packages/beacon-node/src/api/rest/index.ts +++ b/packages/beacon-node/src/api/rest/index.ts @@ -22,6 +22,7 @@ export const beaconRestApiServerOpts: BeaconRestApiServerOpts = { cors: "*", // beacon -> validator API is trusted, and for large amounts of keys the payload is multi-MB bodyLimit: 20 * 1024 * 1024, // 20MB for big block + blobs + stacktraces: false, }; export type BeaconRestApiServerModules = RestApiServerModules & { diff --git a/packages/cli/src/cmds/dev/options.ts b/packages/cli/src/cmds/dev/options.ts index c484150e58d7..5286b81729c6 100644 --- a/packages/cli/src/cmds/dev/options.ts +++ b/packages/cli/src/cmds/dev/options.ts @@ -90,6 +90,10 @@ const externalOptionsOverrides: Partial = { @@ -141,6 +142,12 @@ export const keymanagerOptions: CliCommandOptions = { type: "number", description: "Defines the maximum payload, in bytes, the server is allowed to accept", }, + "keymanager.stacktraces": { + hidden: true, + type: "boolean", + description: "Return stacktraces in HTTP error responses", + group: "api", + }, }; export const validatorOptions: CliCommandOptions = { diff --git a/packages/cli/src/options/beaconNodeOptions/api.ts b/packages/cli/src/options/beaconNodeOptions/api.ts index 996136f262ec..bed0105fd944 100644 --- a/packages/cli/src/options/beaconNodeOptions/api.ts +++ b/packages/cli/src/options/beaconNodeOptions/api.ts @@ -12,6 +12,7 @@ export type ApiArgs = { "rest.port": number; "rest.headerLimit"?: number; "rest.bodyLimit"?: number; + "rest.stacktraces"?: boolean; "rest.swaggerUI"?: boolean; }; @@ -26,6 +27,7 @@ export function parseArgs(args: ApiArgs): IBeaconNodeOptions["api"] { port: args["rest.port"], headerLimit: args["rest.headerLimit"], bodyLimit: args["rest.bodyLimit"], + stacktraces: args["rest.stacktraces"], swaggerUI: args["rest.swaggerUI"], }, }; @@ -92,6 +94,13 @@ export const options: CliCommandOptions = { description: "Defines the maximum payload, in bytes, the server is allowed to accept", }, + "rest.stacktraces": { + hidden: true, + type: "boolean", + description: "Return stacktraces in HTTP error responses", + group: "api", + }, + "rest.swaggerUI": { type: "boolean", description: "Enable Swagger UI for API exploration at http://{address}:{port}/documentation", diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index 07cd5fec0cc4..264570636fad 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import assert from "node:assert"; import {toHexString} from "@chainsafe/ssz"; -import {routes} from "@lodestar/api"; +import {routes, fetch} from "@lodestar/api"; import {Simulation} from "../utils/crucible/simulation.js"; import {BeaconClient, ExecutionClient} from "../utils/crucible/interfaces.js"; import {defineSimTestConfig, logFilesDir} from "../utils/crucible/utils/index.js"; @@ -105,6 +105,20 @@ await env.tracker.assert( } ); +await env.tracker.assert("should return HTTP error responses in a spec compliant format", async () => { + // ApiError with status 400 is thrown by handler + const res1 = await node.api.beacon.getStateValidator({stateId: "current", validatorId: 1}); + assert.equal(res1.json(), {code: 400, message: "Invalid block id 'current'"}); + + // JSON schema validation failed" + const res2 = await node.api.beacon.getPoolAttestationsV2({slot: "current" as unknown as number, committeeIndex: 123}); + assert.equal(res2.json(), {code: 400, message: "slot must be integer"}); + + // Route does not exist + const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); + assert.equal(res3.json(), {code: 404, message: "Route GET:/not/implemented/route not found"}); +}); + await env.tracker.assert("BN Not Synced", async () => { const expectedSyncStatus: routes.node.SyncingStatus = { headSlot: 2, diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index d74ae73b966f..f873102edf74 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -17,6 +17,7 @@ describe("options / beaconNodeOptions", () => { "rest.port": 7654, "rest.headerLimit": 16384, "rest.bodyLimit": 30e6, + "rest.stacktraces": true, "chain.blsVerifyAllMultiThread": true, "chain.blsVerifyAllMainThread": true, @@ -122,6 +123,7 @@ describe("options / beaconNodeOptions", () => { port: 7654, headerLimit: 16384, bodyLimit: 30e6, + stacktraces: true, }, }, chain: { From 0e2a0990bd816b7be58daa52c048e8f136c25995 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 26 Sep 2024 17:49:19 +0100 Subject: [PATCH 2/9] Fix error assertions --- packages/cli/test/sim/endpoints.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index 264570636fad..13caa230d90e 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -108,15 +108,15 @@ await env.tracker.assert( await env.tracker.assert("should return HTTP error responses in a spec compliant format", async () => { // ApiError with status 400 is thrown by handler const res1 = await node.api.beacon.getStateValidator({stateId: "current", validatorId: 1}); - assert.equal(res1.json(), {code: 400, message: "Invalid block id 'current'"}); + assert.equal(JSON.parse(await res1.text()), {code: 400, message: "Invalid block id 'current'"}); - // JSON schema validation failed" + // JSON schema validation failed const res2 = await node.api.beacon.getPoolAttestationsV2({slot: "current" as unknown as number, committeeIndex: 123}); - assert.equal(res2.json(), {code: 400, message: "slot must be integer"}); + assert.equal(JSON.parse(await res2.text()), {code: 400, message: "slot must be integer"}); // Route does not exist const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); - assert.equal(res3.json(), {code: 404, message: "Route GET:/not/implemented/route not found"}); + assert.equal(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); }); await env.tracker.assert("BN Not Synced", async () => { From fd1e4f804d1d3acad72e6710d1d22ff436199b55 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 26 Sep 2024 17:51:29 +0100 Subject: [PATCH 3/9] Remove group tag from keymanager option --- packages/cli/src/cmds/validator/options.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/cmds/validator/options.ts b/packages/cli/src/cmds/validator/options.ts index 776bc329d3b0..87b43543b62e 100644 --- a/packages/cli/src/cmds/validator/options.ts +++ b/packages/cli/src/cmds/validator/options.ts @@ -146,7 +146,6 @@ export const keymanagerOptions: CliCommandOptions = { hidden: true, type: "boolean", description: "Return stacktraces in HTTP error responses", - group: "api", }, }; From 9d99de434cdebc45148f7c155194ea46df300889 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 26 Sep 2024 18:46:35 +0100 Subject: [PATCH 4/9] Fix body has already been read --- packages/cli/test/sim/endpoints.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index 13caa230d90e..ea9506d5e1a1 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -108,11 +108,11 @@ await env.tracker.assert( await env.tracker.assert("should return HTTP error responses in a spec compliant format", async () => { // ApiError with status 400 is thrown by handler const res1 = await node.api.beacon.getStateValidator({stateId: "current", validatorId: 1}); - assert.equal(JSON.parse(await res1.text()), {code: 400, message: "Invalid block id 'current'"}); + assert.equal(JSON.parse(await res1.errorBody()), {code: 400, message: "Invalid block id 'current'"}); // JSON schema validation failed const res2 = await node.api.beacon.getPoolAttestationsV2({slot: "current" as unknown as number, committeeIndex: 123}); - assert.equal(JSON.parse(await res2.text()), {code: 400, message: "slot must be integer"}); + assert.equal(JSON.parse(await res2.errorBody()), {code: 400, message: "slot must be integer"}); // Route does not exist const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); From a150a95a03f2b7a1735c4f4c0be7919be44a0b2f Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 26 Sep 2024 19:03:17 +0100 Subject: [PATCH 5/9] Assert deepEqual --- packages/cli/test/sim/endpoints.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index ea9506d5e1a1..ed8ea0607cc1 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -108,15 +108,15 @@ await env.tracker.assert( await env.tracker.assert("should return HTTP error responses in a spec compliant format", async () => { // ApiError with status 400 is thrown by handler const res1 = await node.api.beacon.getStateValidator({stateId: "current", validatorId: 1}); - assert.equal(JSON.parse(await res1.errorBody()), {code: 400, message: "Invalid block id 'current'"}); + assert.deepEqual(JSON.parse(await res1.errorBody()), {code: 400, message: "Invalid block id 'current'"}); // JSON schema validation failed const res2 = await node.api.beacon.getPoolAttestationsV2({slot: "current" as unknown as number, committeeIndex: 123}); - assert.equal(JSON.parse(await res2.errorBody()), {code: 400, message: "slot must be integer"}); + assert.deepEqual(JSON.parse(await res2.errorBody()), {code: 400, message: "slot must be integer"}); // Route does not exist const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); - assert.equal(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); + assert.deepEqual(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); }); await env.tracker.assert("BN Not Synced", async () => { From 98e6427e8a4a1cae42e8bfb2db23c550bdcb76b5 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 27 Sep 2024 12:25:32 +0100 Subject: [PATCH 6/9] fix: update multiple api errors to be spec compliant --- packages/api/src/utils/client/response.ts | 5 ++- .../src/api/impl/beacon/pool/index.ts | 32 ++++++++----------- packages/beacon-node/src/api/impl/errors.ts | 13 ++++++++ packages/beacon-node/src/api/rest/base.ts | 14 +++++++- packages/cli/test/sim/endpoints.test.ts | 9 ++++++ 5 files changed, 52 insertions(+), 21 deletions(-) diff --git a/packages/api/src/utils/client/response.ts b/packages/api/src/utils/client/response.ts index ed008273588a..fdcb2afda943 100644 --- a/packages/api/src/utils/client/response.ts +++ b/packages/api/src/utils/client/response.ts @@ -189,8 +189,11 @@ export class ApiResponse extends Response { private getErrorMessage(): string { const errBody = this.resolvedErrorBody(); try { - const errJson = JSON.parse(errBody) as {message?: string}; + const errJson = JSON.parse(errBody) as {message?: string; failures?: {message: string}[]}; if (errJson.message) { + if (errJson.failures) { + return `${errJson.message}\n` + errJson.failures.map((e) => e.message).join("\n"); + } return errJson.message; } else { return errBody; diff --git a/packages/beacon-node/src/api/impl/beacon/pool/index.ts b/packages/beacon-node/src/api/impl/beacon/pool/index.ts index 398238aa2508..e01b172f1e72 100644 --- a/packages/beacon-node/src/api/impl/beacon/pool/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/pool/index.ts @@ -16,7 +16,7 @@ import { SyncCommitteeError, } from "../../../../chain/errors/index.js"; import {validateGossipFnRetryUnknownRoot} from "../../../../network/processor/gossipHandlers.js"; -import {ApiError} from "../../errors.js"; +import {ApiError, FailureList, IndexedError} from "../../errors.js"; export function getBeaconPoolApi({ chain, @@ -88,7 +88,7 @@ export function getBeaconPoolApi({ async submitPoolAttestationsV2({signedAttestations}) { const seenTimestampSec = Date.now() / 1000; - const errors: Error[] = []; + const failures: FailureList = []; await Promise.all( signedAttestations.map(async (attestation, i) => { @@ -127,7 +127,7 @@ export function getBeaconPoolApi({ return; } - errors.push(e as Error); + failures.push({index: i, message: (e as Error).message}); logger.error(`Error on submitPoolAttestations [${i}]`, logCtx, e as Error); if (e instanceof AttestationError && e.action === GossipAction.REJECT) { chain.persistInvalidSszValue(ssz.phase0.Attestation, attestation, "api_reject"); @@ -136,10 +136,8 @@ export function getBeaconPoolApi({ }) ); - if (errors.length > 1) { - throw Error("Multiple errors on submitPoolAttestations\n" + errors.map((e) => e.message).join("\n")); - } else if (errors.length === 1) { - throw errors[0]; + if (failures.length > 0) { + throw new IndexedError("Error processing attestations", failures); } }, @@ -168,7 +166,7 @@ export function getBeaconPoolApi({ }, async submitPoolBLSToExecutionChange({blsToExecutionChanges}) { - const errors: Error[] = []; + const failures: FailureList = []; await Promise.all( blsToExecutionChanges.map(async (blsToExecutionChange, i) => { @@ -184,7 +182,7 @@ export function getBeaconPoolApi({ await network.publishBlsToExecutionChange(blsToExecutionChange); } } catch (e) { - errors.push(e as Error); + failures.push({index: i, message: (e as Error).message}); logger.error( `Error on submitPoolBLSToExecutionChange [${i}]`, {validatorIndex: blsToExecutionChange.message.validatorIndex}, @@ -194,10 +192,8 @@ export function getBeaconPoolApi({ }) ); - if (errors.length > 1) { - throw Error("Multiple errors on submitPoolBLSToExecutionChange\n" + errors.map((e) => e.message).join("\n")); - } else if (errors.length === 1) { - throw errors[0]; + if (failures.length > 0) { + throw new IndexedError("Error processing BLS to execution changes", failures); } }, @@ -221,7 +217,7 @@ export function getBeaconPoolApi({ // TODO: Fetch states at signature slots const state = chain.getHeadState(); - const errors: Error[] = []; + const failures: FailureList = []; await Promise.all( signatures.map(async (signature, i) => { @@ -261,7 +257,7 @@ export function getBeaconPoolApi({ return; } - errors.push(e as Error); + failures.push({index: i, message: (e as Error).message}); logger.debug( `Error on submitPoolSyncCommitteeSignatures [${i}]`, {slot: signature.slot, validatorIndex: signature.validatorIndex}, @@ -274,10 +270,8 @@ export function getBeaconPoolApi({ }) ); - if (errors.length > 1) { - throw Error("Multiple errors on submitPoolSyncCommitteeSignatures\n" + errors.map((e) => e.message).join("\n")); - } else if (errors.length === 1) { - throw errors[0]; + if (failures.length > 0) { + throw new IndexedError("Error processing sync committee signatures", failures); } }, }; diff --git a/packages/beacon-node/src/api/impl/errors.ts b/packages/beacon-node/src/api/impl/errors.ts index 848691f7cf6d..609f40f83a12 100644 --- a/packages/beacon-node/src/api/impl/errors.ts +++ b/packages/beacon-node/src/api/impl/errors.ts @@ -35,3 +35,16 @@ export class OnlySupportedByDVT extends ApiError { super(501, "Only supported by distributed validator middleware clients"); } } + +// Error thrown when processing multiple items failed - https://github.com/ethereum/beacon-APIs/blob/e7f7d70423b0abfe9d9f33b701be2ec03e44eb02/types/http.yaml#L175 +export class IndexedError extends ApiError { + failures: FailureList; + + constructor(message: string, failures: FailureList) { + super(400, message); + + this.failures = failures.sort((a, b) => a.index - b.index); + } +} + +export type FailureList = {index: number; message: string}[]; diff --git a/packages/beacon-node/src/api/rest/base.ts b/packages/beacon-node/src/api/rest/base.ts index e8ea85f67af5..10318b1b3ba7 100644 --- a/packages/beacon-node/src/api/rest/base.ts +++ b/packages/beacon-node/src/api/rest/base.ts @@ -5,7 +5,7 @@ import bearerAuthPlugin from "@fastify/bearer-auth"; import {addSszContentTypeParser} from "@lodestar/api/server"; import {ErrorAborted, Gauge, Histogram, Logger} from "@lodestar/utils"; import {isLocalhostIP} from "../../util/ip.js"; -import {ApiError, NodeIsSyncing} from "../impl/errors.js"; +import {ApiError, FailureList, IndexedError, NodeIsSyncing} from "../impl/errors.js"; import {HttpActiveSocketsTracker, SocketMetrics} from "./activeSockets.js"; export type RestApiServerOpts = { @@ -41,6 +41,10 @@ type ErrorResponse = { stacktraces?: string[]; }; +type IndexedErrorResponse = ErrorResponse & { + failures?: FailureList; +}; + /** * Error code used by Fastify if media type is not supported (415) */ @@ -97,6 +101,14 @@ export class RestApiServer { stacktraces, }; void res.status(400).send(payload); + } else if (err instanceof IndexedError) { + const payload: IndexedErrorResponse = { + code: err.statusCode, + message: err.message, + failures: err.failures, + stacktraces, + }; + void res.status(err.statusCode).send(payload); } else { // Convert our custom ApiError into status code const statusCode = err instanceof ApiError ? err.statusCode : 500; diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index ed8ea0607cc1..ae8c54630417 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -3,6 +3,7 @@ import path from "node:path"; import assert from "node:assert"; import {toHexString} from "@chainsafe/ssz"; import {routes, fetch} from "@lodestar/api"; +import {ssz} from "@lodestar/types"; import {Simulation} from "../utils/crucible/simulation.js"; import {BeaconClient, ExecutionClient} from "../utils/crucible/interfaces.js"; import {defineSimTestConfig, logFilesDir} from "../utils/crucible/utils/index.js"; @@ -117,6 +118,14 @@ await env.tracker.assert("should return HTTP error responses in a spec compliant // Route does not exist const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); assert.deepEqual(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); + + // Error processing multiple items + const signedAttestations = Array.from({length: 3}, () => ssz.phase0.Attestation.defaultValue()); + const res4 = await node.api.beacon.submitPoolAttestationsV2({signedAttestations}); + const errBody = JSON.parse(await res4.errorBody()) as {code: number; message: string; failures: unknown[]}; + assert.deepEqual(errBody, {code: 400, message: "Error processing attestations"}); + assert.equal(errBody.failures.length, signedAttestations.length); + assert.deepEqual(errBody.failures[0], {index: 0, message: "ATTESTATION_ERROR_NOT_EXACTLY_ONE_AGGREGATION_BIT_SET"}); }); await env.tracker.assert("BN Not Synced", async () => { From 95ed5d96afd47c0dfbf6c35bf4f44947cf5db723 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 27 Sep 2024 12:42:40 +0100 Subject: [PATCH 7/9] Reorder test cases --- packages/cli/test/sim/endpoints.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index ae8c54630417..2aab0d2a85c3 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -115,10 +115,6 @@ await env.tracker.assert("should return HTTP error responses in a spec compliant const res2 = await node.api.beacon.getPoolAttestationsV2({slot: "current" as unknown as number, committeeIndex: 123}); assert.deepEqual(JSON.parse(await res2.errorBody()), {code: 400, message: "slot must be integer"}); - // Route does not exist - const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); - assert.deepEqual(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); - // Error processing multiple items const signedAttestations = Array.from({length: 3}, () => ssz.phase0.Attestation.defaultValue()); const res4 = await node.api.beacon.submitPoolAttestationsV2({signedAttestations}); @@ -126,6 +122,10 @@ await env.tracker.assert("should return HTTP error responses in a spec compliant assert.deepEqual(errBody, {code: 400, message: "Error processing attestations"}); assert.equal(errBody.failures.length, signedAttestations.length); assert.deepEqual(errBody.failures[0], {index: 0, message: "ATTESTATION_ERROR_NOT_EXACTLY_ONE_AGGREGATION_BIT_SET"}); + + // Route does not exist + const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); + assert.deepEqual(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); }); await env.tracker.assert("BN Not Synced", async () => { From d2ec3fea9a5666498ef66eff51df63e607f96b3a Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 27 Sep 2024 12:43:33 +0100 Subject: [PATCH 8/9] Update res numbering --- packages/cli/test/sim/endpoints.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index 2aab0d2a85c3..92ab9b38fe5c 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -117,15 +117,15 @@ await env.tracker.assert("should return HTTP error responses in a spec compliant // Error processing multiple items const signedAttestations = Array.from({length: 3}, () => ssz.phase0.Attestation.defaultValue()); - const res4 = await node.api.beacon.submitPoolAttestationsV2({signedAttestations}); - const errBody = JSON.parse(await res4.errorBody()) as {code: number; message: string; failures: unknown[]}; + const res3 = await node.api.beacon.submitPoolAttestationsV2({signedAttestations}); + const errBody = JSON.parse(await res3.errorBody()) as {code: number; message: string; failures: unknown[]}; assert.deepEqual(errBody, {code: 400, message: "Error processing attestations"}); assert.equal(errBody.failures.length, signedAttestations.length); assert.deepEqual(errBody.failures[0], {index: 0, message: "ATTESTATION_ERROR_NOT_EXACTLY_ONE_AGGREGATION_BIT_SET"}); // Route does not exist - const res3 = await fetch(`${node.restPublicUrl}/not/implemented/route`); - assert.deepEqual(JSON.parse(await res3.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); + const res4 = await fetch(`${node.restPublicUrl}/not/implemented/route`); + assert.deepEqual(JSON.parse(await res4.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); }); await env.tracker.assert("BN Not Synced", async () => { From 9fff901351471c0c549c439e4af4ce3180930339 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 27 Sep 2024 13:46:26 +0100 Subject: [PATCH 9/9] Use strict deep equal --- packages/cli/test/sim/endpoints.test.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/cli/test/sim/endpoints.test.ts b/packages/cli/test/sim/endpoints.test.ts index 92ab9b38fe5c..356b00ad2df7 100644 --- a/packages/cli/test/sim/endpoints.test.ts +++ b/packages/cli/test/sim/endpoints.test.ts @@ -109,23 +109,30 @@ await env.tracker.assert( await env.tracker.assert("should return HTTP error responses in a spec compliant format", async () => { // ApiError with status 400 is thrown by handler const res1 = await node.api.beacon.getStateValidator({stateId: "current", validatorId: 1}); - assert.deepEqual(JSON.parse(await res1.errorBody()), {code: 400, message: "Invalid block id 'current'"}); + assert.deepStrictEqual(JSON.parse(await res1.errorBody()), {code: 400, message: "Invalid block id 'current'"}); // JSON schema validation failed const res2 = await node.api.beacon.getPoolAttestationsV2({slot: "current" as unknown as number, committeeIndex: 123}); - assert.deepEqual(JSON.parse(await res2.errorBody()), {code: 400, message: "slot must be integer"}); + assert.deepStrictEqual(JSON.parse(await res2.errorBody()), {code: 400, message: "slot must be integer"}); // Error processing multiple items const signedAttestations = Array.from({length: 3}, () => ssz.phase0.Attestation.defaultValue()); const res3 = await node.api.beacon.submitPoolAttestationsV2({signedAttestations}); const errBody = JSON.parse(await res3.errorBody()) as {code: number; message: string; failures: unknown[]}; - assert.deepEqual(errBody, {code: 400, message: "Error processing attestations"}); + assert.equal(errBody.code, 400); + assert.equal(errBody.message, "Error processing attestations"); assert.equal(errBody.failures.length, signedAttestations.length); - assert.deepEqual(errBody.failures[0], {index: 0, message: "ATTESTATION_ERROR_NOT_EXACTLY_ONE_AGGREGATION_BIT_SET"}); + assert.deepStrictEqual(errBody.failures[0], { + index: 0, + message: "ATTESTATION_ERROR_NOT_EXACTLY_ONE_AGGREGATION_BIT_SET", + }); // Route does not exist const res4 = await fetch(`${node.restPublicUrl}/not/implemented/route`); - assert.deepEqual(JSON.parse(await res4.text()), {code: 404, message: "Route GET:/not/implemented/route not found"}); + assert.deepStrictEqual(JSON.parse(await res4.text()), { + code: 404, + message: "Route GET:/not/implemented/route not found", + }); }); await env.tracker.assert("BN Not Synced", async () => {