From 1d3c39279c3d476dd4bf0fdb32576eb4b5d1ca9e Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 23 Jul 2026 19:08:52 +0100 Subject: [PATCH 1/2] fix: return 400 for gossip validation errors --- packages/beacon-node/src/api/rest/base.ts | 11 ++-- .../test/unit/api/rest/base.test.ts | 53 +++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 packages/beacon-node/test/unit/api/rest/base.test.ts diff --git a/packages/beacon-node/src/api/rest/base.ts b/packages/beacon-node/src/api/rest/base.ts index 7589720de166..e82e25393ad4 100644 --- a/packages/beacon-node/src/api/rest/base.ts +++ b/packages/beacon-node/src/api/rest/base.ts @@ -5,6 +5,7 @@ import {parse as parseQueryString} from "qs"; import {addSszContentTypeParser} from "@lodestar/api/server"; import {NUMBER_OF_COLUMNS} from "@lodestar/params"; import {ErrorAborted, Gauge, Histogram, Logger} from "@lodestar/utils"; +import {GossipActionError} from "../../chain/errors/gossipValidation.js"; import {isLocalhostIP} from "../../util/ip.js"; import {ApiError, FailureList, IndexedError, NodeIsSyncing} from "../impl/errors.js"; import {HttpActiveSocketsTracker, SocketMetrics} from "./activeSockets.js"; @@ -121,8 +122,8 @@ export class RestApiServer { }; void res.status(err.statusCode).send(payload); } else { - // Convert our custom ApiError into status code - const statusCode = err instanceof ApiError ? err.statusCode : 500; + // Convert known request errors into status codes + const statusCode = err instanceof ApiError ? err.statusCode : err instanceof GossipActionError ? 400 : 500; const payload: ErrorResponse = {code: statusCode, message: err.message, stacktraces}; void res.status(statusCode).send(payload); } @@ -170,7 +171,11 @@ export class RestApiServer { const operationId = getOperationId(req); - if (err instanceof ApiError || [INVALID_MEDIA_TYPE_CODE, SCHEMA_VALIDATION_ERROR_CODE].includes(err.code)) { + if ( + err instanceof ApiError || + err instanceof GossipActionError || + [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/test/unit/api/rest/base.test.ts b/packages/beacon-node/test/unit/api/rest/base.test.ts new file mode 100644 index 000000000000..54b6c5939931 --- /dev/null +++ b/packages/beacon-node/test/unit/api/rest/base.test.ts @@ -0,0 +1,53 @@ +import {afterEach, describe, expect, it} from "vitest"; +import {RestApiServer} from "../../../../src/api/rest/base.js"; +import {GossipAction, GossipActionError} from "../../../../src/chain/errors/gossipValidation.js"; +import {getMockedLogger} from "../../../mocks/loggerMock.js"; + +class TestRestApiServer extends RestApiServer { + registerErrorRoute(error: Error): void { + this.server.get("/error", async () => { + throw error; + }); + } + + async getErrorResponse(): Promise<{statusCode: number; json: () => unknown}> { + return this.server.inject({method: "GET", url: "/error"}); + } +} + +describe("RestApiServer", () => { + let server: TestRestApiServer | undefined; + + afterEach(async () => { + await server?.close(); + }); + + it.each([GossipAction.IGNORE, GossipAction.REJECT])( + "returns 400 for a %s gossip validation error", + async (action) => { + server = new TestRestApiServer({port: 0}, {logger: getMockedLogger(), metrics: null}); + server.registerErrorRoute(new GossipActionError(action, {code: "TEST_GOSSIP_VALIDATION_ERROR"})); + + const response = await server.getErrorResponse(); + + expect(response.statusCode).toBe(400); + expect(response.json()).toEqual({ + code: 400, + message: "TEST_GOSSIP_VALIDATION_ERROR", + }); + } + ); + + it("returns 500 for an unexpected error", async () => { + server = new TestRestApiServer({port: 0}, {logger: getMockedLogger(), metrics: null}); + server.registerErrorRoute(new Error("Unexpected error")); + + const response = await server.getErrorResponse(); + + expect(response.statusCode).toBe(500); + expect(response.json()).toEqual({ + code: 500, + message: "Unexpected error", + }); + }); +}); From ad6d27656a48e8d33138ae71c16b05c71ff8defc Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 23 Jul 2026 19:14:28 +0100 Subject: [PATCH 2/2] test: remove REST error handler tests --- .../test/unit/api/rest/base.test.ts | 53 ------------------- 1 file changed, 53 deletions(-) delete mode 100644 packages/beacon-node/test/unit/api/rest/base.test.ts diff --git a/packages/beacon-node/test/unit/api/rest/base.test.ts b/packages/beacon-node/test/unit/api/rest/base.test.ts deleted file mode 100644 index 54b6c5939931..000000000000 --- a/packages/beacon-node/test/unit/api/rest/base.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import {afterEach, describe, expect, it} from "vitest"; -import {RestApiServer} from "../../../../src/api/rest/base.js"; -import {GossipAction, GossipActionError} from "../../../../src/chain/errors/gossipValidation.js"; -import {getMockedLogger} from "../../../mocks/loggerMock.js"; - -class TestRestApiServer extends RestApiServer { - registerErrorRoute(error: Error): void { - this.server.get("/error", async () => { - throw error; - }); - } - - async getErrorResponse(): Promise<{statusCode: number; json: () => unknown}> { - return this.server.inject({method: "GET", url: "/error"}); - } -} - -describe("RestApiServer", () => { - let server: TestRestApiServer | undefined; - - afterEach(async () => { - await server?.close(); - }); - - it.each([GossipAction.IGNORE, GossipAction.REJECT])( - "returns 400 for a %s gossip validation error", - async (action) => { - server = new TestRestApiServer({port: 0}, {logger: getMockedLogger(), metrics: null}); - server.registerErrorRoute(new GossipActionError(action, {code: "TEST_GOSSIP_VALIDATION_ERROR"})); - - const response = await server.getErrorResponse(); - - expect(response.statusCode).toBe(400); - expect(response.json()).toEqual({ - code: 400, - message: "TEST_GOSSIP_VALIDATION_ERROR", - }); - } - ); - - it("returns 500 for an unexpected error", async () => { - server = new TestRestApiServer({port: 0}, {logger: getMockedLogger(), metrics: null}); - server.registerErrorRoute(new Error("Unexpected error")); - - const response = await server.getErrorResponse(); - - expect(response.statusCode).toBe(500); - expect(response.json()).toEqual({ - code: 500, - message: "Unexpected error", - }); - }); -});