From b6501c6ba6d5f8dfb481528550651157a4eb804c Mon Sep 17 00:00:00 2001 From: Etienne Donneger Date: Wed, 1 May 2024 17:20:27 -0400 Subject: [PATCH] Add consistent error codes --- src/fetch/GET.ts | 8 ++------ src/fetch/balance.ts | 19 +++++++++++++++---- src/fetch/head.ts | 17 +++++++++++------ src/fetch/health.ts | 4 ++-- src/fetch/openapi.ts | 29 +++++++++++++++++++++++++---- src/fetch/supply.ts | 19 +++++++++++++++---- src/fetch/transfers.ts | 12 +++++++++--- src/fetch/utils.spec.ts | 10 +++++----- src/fetch/utils.ts | 24 ++++++++++++++++++++++-- 9 files changed, 106 insertions(+), 36 deletions(-) diff --git a/src/fetch/GET.ts b/src/fetch/GET.ts index 40f78dd..4a0a12a 100644 --- a/src/fetch/GET.ts +++ b/src/fetch/GET.ts @@ -5,11 +5,10 @@ import head from "./head.js"; import balance from "./balance.js"; import supply from "./supply.js"; import * as prometheus from "../prometheus.js"; -import { logger } from "../logger.js"; import swaggerHtml from "../../swagger/index.html" import swaggerFavicon from "../../swagger/favicon.png" import transfers from "./transfers.js"; -import { toJSON } from "./utils.js"; +import { APIError, toJSON } from "./utils.js"; import { APP_VERSION } from "../config.js"; export default async function (req: Request) { @@ -32,8 +31,5 @@ export default async function (req: Request) { if (pathname === "/balance") return balance(req); if (pathname === "/transfers") return transfers(req); - logger.warn(`Not found: ${pathname}`); - prometheus.request_error.inc({ pathname, status: 404 }); - - return new Response("Not found", { status: 404 }); + return APIError(pathname, 404, "path_not_found", "Invalid pathname"); } diff --git a/src/fetch/balance.ts b/src/fetch/balance.ts index 56172d9..6c6f446 100644 --- a/src/fetch/balance.ts +++ b/src/fetch/balance.ts @@ -2,7 +2,7 @@ import { makeQuery } from "../clickhouse/makeQuery.js"; import { logger } from "../logger.js"; import { getBalanceChanges } from "../queries.js"; import * as prometheus from "../prometheus.js"; -import { addMetadata, toJSON } from "./utils.js"; +import { APIError, addMetadata, toJSON } from "./utils.js"; import { parseLimit, parsePage } from "../utils.js"; function verifyParams(searchParams: URLSearchParams) { @@ -14,12 +14,23 @@ function verifyParams(searchParams: URLSearchParams) { export default async function (req: Request) { try { - const { searchParams } = new URL(req.url); + const { pathname, searchParams } = new URL(req.url); logger.info({ searchParams: Object.fromEntries(Array.from(searchParams)) }); - verifyParams(searchParams); + try { + verifyParams(searchParams); + } catch (e: any) { + return APIError(pathname, 400, "bad_query_input", e.message); + } + const query = getBalanceChanges(searchParams); - const response = await makeQuery(query) + let response; + + try { + response = await makeQuery(query); + } catch (e: any) { + return APIError(pathname, 500, "failed_database_query", e.message); + } return toJSON( addMetadata( diff --git a/src/fetch/head.ts b/src/fetch/head.ts index 0a56bac..378324e 100644 --- a/src/fetch/head.ts +++ b/src/fetch/head.ts @@ -1,10 +1,15 @@ -import { addMetadata, toJSON } from "./utils.js"; +import { APIError, addMetadata, toJSON } from "./utils.js"; import { makeQuery } from "../clickhouse/makeQuery.js"; export default async function (req: Request) { - return toJSON( - addMetadata( - await makeQuery("SELECT block_num FROM cursors ORDER BY block_num DESC LIMIT 1") - ) - ); + let query = "SELECT block_num FROM cursors ORDER BY block_num DESC LIMIT 1"; + let response; + + try { + response = await makeQuery(query); + } catch (e: any) { + return APIError(new URL(req.url).pathname, 500, "failed_database_query", e.message); + } + + return toJSON(addMetadata(response)); } \ No newline at end of file diff --git a/src/fetch/health.ts b/src/fetch/health.ts index 24719a5..af86916 100644 --- a/src/fetch/health.ts +++ b/src/fetch/health.ts @@ -13,8 +13,8 @@ export default async function (_req: Request) { return new Response("Unknown response from ClickHouse"); } catch (e: any) { logger.error(e); - prometheus.request_error.inc({ pathname: "/health", status: 500 }); + prometheus.request_error.inc({ pathname: "/health", status: 503 }); - return new Response(e.message, { status: 500 }); + return new Response(e.message, { status: 503 }); } } \ No newline at end of file diff --git a/src/fetch/openapi.ts b/src/fetch/openapi.ts index 2881d36..5f2684d 100644 --- a/src/fetch/openapi.ts +++ b/src/fetch/openapi.ts @@ -5,7 +5,7 @@ import { config } from "../config.js"; import { registry } from "../prometheus.js"; import { makeQuery } from "../clickhouse/makeQuery.js"; import { getBalanceChanges, getTotalSupply, getTransfers } from "../queries.js"; -import { addMetadata } from "./utils.js"; +import { APIError, addMetadata } from "./utils.js"; const TAGS = { MONITORING: "Monitoring", HEALTH: "Health", @@ -28,9 +28,30 @@ const head_example = addMetadata({ bytes_read: 32 } }); -const supply_example = await makeQuery(getTotalSupply(new URLSearchParams({ limit: "1" }), true)).then(res => addMetadata(res, 1, 1)); -const balance_example = await makeQuery(getBalanceChanges(new URLSearchParams({ limit: "2" }), true)).then(res => addMetadata(res, 2, 1)); -const transfers_example = await makeQuery(getTransfers(new URLSearchParams({ limit: "5" }), true)).then(res => addMetadata(res, 5, 1)); + +const supply_example = await makeQuery( + getTotalSupply(new URLSearchParams({ limit: "1" }), true) +).then( + res => addMetadata(res, 1, 1) +).catch( + e => APIError("/openapi", 500, "failed_database_query", e.message) +); + +const balance_example = await makeQuery( + getBalanceChanges(new URLSearchParams({ limit: "2" }), true) +).then( + res => addMetadata(res, 2, 1) +).catch( + e => APIError("/openapi", 500, "failed_database_query", e.message) +); + +const transfers_example = await makeQuery( + getTotalSupply(new URLSearchParams({ limit: "5" }), true) +).then( + res => addMetadata(res, 5, 1) +).catch( + e => APIError("/openapi", 500, "failed_database_query", e.message) +); const timestampSchema: SchemaObject = { anyOf: [ diff --git a/src/fetch/supply.ts b/src/fetch/supply.ts index 2838e78..b4beb10 100644 --- a/src/fetch/supply.ts +++ b/src/fetch/supply.ts @@ -2,7 +2,7 @@ import { makeQuery } from "../clickhouse/makeQuery.js"; import { logger } from "../logger.js"; import { getTotalSupply } from "../queries.js"; import * as prometheus from "../prometheus.js"; -import { addMetadata, toJSON } from "./utils.js"; +import { APIError, addMetadata, toJSON } from "./utils.js"; import { parseLimit, parsePage } from "../utils.js"; function verifyParams(searchParams: URLSearchParams) { @@ -14,12 +14,23 @@ function verifyParams(searchParams: URLSearchParams) { export default async function (req: Request) { try { - const { searchParams } = new URL(req.url); + const { pathname, searchParams } = new URL(req.url); logger.info({ searchParams: Object.fromEntries(Array.from(searchParams)) }); - verifyParams(searchParams); + try { + verifyParams(searchParams); + } catch (e: any) { + return APIError(pathname, 400, "bad_query_input", e.message); + } + const query = getTotalSupply(searchParams); - const response = await makeQuery(query) + let response; + + try { + response = await makeQuery(query); + } catch (e: any) { + return APIError(pathname, 500, "failed_database_query", e.message); + } return toJSON( addMetadata( diff --git a/src/fetch/transfers.ts b/src/fetch/transfers.ts index 26aaeb5..73f506b 100644 --- a/src/fetch/transfers.ts +++ b/src/fetch/transfers.ts @@ -2,16 +2,22 @@ import { makeQuery } from "../clickhouse/makeQuery.js"; import { logger } from "../logger.js"; import { getTransfers } from "../queries.js"; import * as prometheus from "../prometheus.js"; -import { addMetadata, toJSON } from "./utils.js"; +import { APIError, addMetadata, toJSON } from "./utils.js"; import { parseLimit, parsePage } from "../utils.js"; export default async function (req: Request) { try { - const { searchParams } = new URL(req.url); + const { pathname, searchParams } = new URL(req.url); logger.info({ searchParams: Object.fromEntries(Array.from(searchParams)) }); const query = getTransfers(searchParams); - const response = await makeQuery(query) + let response; + + try { + response = await makeQuery(query); + } catch (e: any) { + return APIError(pathname, 500, "failed_database_query", e.message); + } return toJSON( addMetadata( diff --git a/src/fetch/utils.spec.ts b/src/fetch/utils.spec.ts index 220ac6d..51f6588 100644 --- a/src/fetch/utils.spec.ts +++ b/src/fetch/utils.spec.ts @@ -7,7 +7,7 @@ const mock_query_reponse: Query = { meta: [], data: Array(limit), rows: limit, - rows_before_limit_at_least: 5*limit, // Simulate query with more total results than the query limit making pagination relevant + rows_before_limit_at_least: 5 * limit, // Simulate query with more total results than the query limit making pagination relevant statistics: { elapsed: 0, rows_read: 0, @@ -20,26 +20,26 @@ test("addMetadata pagination", () => { expect(first_page.meta.next_page).toBe(2); expect(first_page.meta.previous_page).toBe(1); // Previous page should be set to 1 on first page expect(first_page.meta.total_pages).toBe(5); - expect(first_page.meta.total_results).toBe(5*limit); + expect(first_page.meta.total_results).toBe(5 * limit); const odd_page = addMetadata(mock_query_reponse, limit, 3); expect(odd_page.meta.next_page).toBe(4); expect(odd_page.meta.previous_page).toBe(2); expect(odd_page.meta.total_pages).toBe(5); - expect(odd_page.meta.total_results).toBe(5*limit); + expect(odd_page.meta.total_results).toBe(5 * limit); const even_page = addMetadata(mock_query_reponse, limit, 4); expect(even_page.meta.next_page).toBe(5); expect(even_page.meta.previous_page).toBe(3); expect(even_page.meta.total_pages).toBe(5); - expect(even_page.meta.total_results).toBe(5*limit); + expect(even_page.meta.total_results).toBe(5 * limit); const last_page = addMetadata(mock_query_reponse, limit, 5); // @ts-ignore expect(last_page.meta.next_page).toBe(last_page.meta.total_pages); // Next page should be capped to total_pages on last page expect(last_page.meta.previous_page).toBe(4); expect(last_page.meta.total_pages).toBe(5); - expect(last_page.meta.total_results).toBe(5*limit); + expect(last_page.meta.total_results).toBe(5 * limit); // TODO: Expect error message on beyond last page // const beyond_last_page = addMetadata(mock_query_reponse.data, mock_query_reponse.rows_before_limit_at_least, limit, 6); diff --git a/src/fetch/utils.ts b/src/fetch/utils.ts index 5195632..5e1f83b 100644 --- a/src/fetch/utils.ts +++ b/src/fetch/utils.ts @@ -1,4 +1,24 @@ import { Query } from "../clickhouse/makeQuery.js"; +import { logger } from "../logger.js"; +import * as prometheus from "../prometheus.js"; + +interface APIError { + status: number, + code?: string, + detail?: string +} + +export function APIError(pathname: string, status: number, code?: string, detail?: string) { + const api_error: APIError = { + status, + code: code ? code : "unknown", + detail: detail ? detail : "" + } + + logger.error(api_error); + prometheus.request_error.inc({ pathname, status }); + return toJSON(api_error, status); +} export function toJSON(data: any, status: number = 200) { return new Response(JSON.stringify(data), { status, headers: { "Content-Type": "application/json" } }); @@ -6,14 +26,14 @@ export function toJSON(data: any, status: number = 200) { export function addMetadata(response: Query, req_limit?: number, req_page?: number) { // TODO: Catch page number greater than total_pages and return error - if (typeof(req_limit) !== 'undefined' && typeof(req_page) !== 'undefined') + if (typeof (req_limit) !== 'undefined' && typeof (req_page) !== 'undefined') return { data: response.data, meta: { statistics: response.statistics, "next_page": (req_page * req_limit >= response.rows_before_limit_at_least) ? req_page : req_page + 1, "previous_page": (req_page <= 1) ? req_page : req_page - 1, - "total_pages": Math.ceil( response.rows_before_limit_at_least / req_limit), + "total_pages": Math.ceil(response.rows_before_limit_at_least / req_limit), "total_results": response.rows_before_limit_at_least } }