From ab485a99b346ab7e7cf9bf21ae17c9e7288c290e Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 17 Dec 2024 09:37:01 +0000 Subject: [PATCH] error endpoint test, lint, grey country border for test page --- src/errors/asyncControllerHandler.ts | 2 +- src/errors/handleError.ts | 5 +- src/routes.ts | 4 +- test.html | 2 +- tests/integration/errors.spec.ts | 46 ++++++++++++------- .../unit/controllers/indexController.spec.ts | 5 +- tests/unit/errors/handleError.spec.ts | 23 +++++++--- tests/unit/errors/notFound.spec.ts | 4 +- tests/unit/jsonResponse.spec.ts | 24 ++++++---- tests/unit/logging.spec.ts | 2 +- tests/unit/routes.spec.ts | 2 +- 11 files changed, 79 insertions(+), 40 deletions(-) diff --git a/src/errors/asyncControllerHandler.ts b/src/errors/asyncControllerHandler.ts index 50a8f61..8500c4d 100644 --- a/src/errors/asyncControllerHandler.ts +++ b/src/errors/asyncControllerHandler.ts @@ -1,7 +1,7 @@ import { NextFunction } from "express"; // This method should be used to wrap any async controller methods to ensure error handling is applied -export default async (next: NextFunction, method: Function) => { +export default async (next: NextFunction, method: () => void) => { try { await method(); } catch (error) { diff --git a/src/errors/handleError.ts b/src/errors/handleError.ts index a89fad0..07e0008 100644 --- a/src/errors/handleError.ts +++ b/src/errors/handleError.ts @@ -1,15 +1,16 @@ -import { Request, Response } from "express"; +import { NextFunction, Request, Response } from "express"; import { uid } from "uid"; import { GroutError } from "./groutError"; import { ErrorType } from "./errorType"; import { reqWithError } from "../logging"; import { jsonResponseError } from "../jsonResponse"; +// We need to include the unused next var for this to be used correctly as an error handler export const handleError = ( err: Error, req: Request, res: Response, - _: Function + _: NextFunction // eslint-disable-line @typescript-eslint/no-unused-vars ) => { const groutError = err instanceof GroutError; diff --git a/src/routes.ts b/src/routes.ts index 4e9977a..b23e6cb 100644 --- a/src/routes.ts +++ b/src/routes.ts @@ -11,7 +11,9 @@ export const registerRoutes = () => { // provide an endpoint we can use to test 500 response behaviour by throwing an "unexpected error" - but only if we // are running in a non-production mode indicated by an env var if (process.env.GROUT_ERROR_TEST) { - router.get("/error-test", () => { throw Error("Testing error behaviour"); }); + router.get("/error-test", () => { + throw Error("Testing error behaviour"); + }); } // Throw 404 error for any unmatched routes diff --git a/test.html b/test.html index 09d4173..fc2b433 100644 --- a/test.html +++ b/test.html @@ -79,7 +79,7 @@

GROUT

style: { weight: 1, fill: false, - color: "#000000" + color: "#777" }, ...options }; diff --git a/tests/integration/errors.spec.ts b/tests/integration/errors.spec.ts index 4bf7b79..61e8fee 100644 --- a/tests/integration/errors.spec.ts +++ b/tests/integration/errors.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, test, beforeAll, afterAll } from "vitest"; -import {grout} from "./integrationTest"; +import { grout } from "./integrationTest"; import * as fs from "fs"; const expect404Response = async (url: string) => { @@ -8,21 +8,17 @@ const expect404Response = async (url: string) => { expect(response.body).toStrictEqual({ status: "failure", data: null, - errors: [ - { error: "NOT_FOUND", detail: `Route not found: ${url}` } - ] + errors: [{ error: "NOT_FOUND", detail: `Route not found: ${url}` }] }); }; -const expect400Response = async(url: string, error: string) => { +const expect400Response = async (url: string, error: string) => { const response = await grout.get(url); expect(response.status).toBe(400); expect(response.body).toStrictEqual({ status: "failure", data: null, - errors: [ - { error: "BAD_REQUEST", detail: error } - ] + errors: [{ error: "BAD_REQUEST", detail: error }] }); }; @@ -42,25 +38,43 @@ describe("404 error response", () => { test("from unknown tile", async () => { await expect404Response("/tile/gadm41/admin0/3/1/4000"); }); - }); describe("400 error response", () => { test("from non-integer z value", async () => { - await expect400Response("/tile/gadm41/admin0/a/1/4", "\"a\" is not an integer"); + await expect400Response( + "/tile/gadm41/admin0/a/1/4", + '"a" is not an integer' + ); }); test("from non-integer x value", async () => { - await expect400Response("/tile/gadm41/admin0/3/1a/4", "\"1a\" is not an integer"); + await expect400Response( + "/tile/gadm41/admin0/3/1a/4", + '"1a" is not an integer' + ); }); test("from non-integer y value", async () => { - await expect400Response("/tile/gadm41/admin0/3/1/4.3", "\"4.3\" is not an integer"); + await expect400Response( + "/tile/gadm41/admin0/3/1/4.3", + '"4.3" is not an integer' + ); }); -}) +}); describe("500 error response", () => { - test("when db file is inaccessible", async () => { - await expect400Response("/tile/gadm41/admin1/3/1/4", "\"4.3\" is not an integer"); + test("when request error test endpoint", async () => { + // NB The error-test endpoint is only enabled when the GROUT_ERROR_TEST env var is set when the server starts + // up, e.g. when running through ./docker/run + const response = await grout.get("/error-test"); + expect(response.status).toBe(500); + expect(response.body.status).toBe("failure"); + expect(response.body.data).toBe(null); + expect(response.body.errors.length).toBe(1); + expect(response.body.errors[0].error).toBe("UNEXPECTED_ERROR"); + expect(response.body.errors[0].detail).toMatch( + /^An unexpected error occurred. Please contact support and quote error code [0-9a-f]{11}$/ + ); }); -}); \ No newline at end of file +}); diff --git a/tests/unit/controllers/indexController.spec.ts b/tests/unit/controllers/indexController.spec.ts index f56f466..d3468f6 100644 --- a/tests/unit/controllers/indexController.spec.ts +++ b/tests/unit/controllers/indexController.spec.ts @@ -15,6 +15,9 @@ describe("IndexController", () => { test("returns expected content and status", () => { const mockResponse = {}; IndexController.getIndex({}, mockResponse); - expect(mockJsonResponseSuccess).toHaveBeenCalledWith({ version: "1.2.3" }, mockResponse); + expect(mockJsonResponseSuccess).toHaveBeenCalledWith( + { version: "1.2.3" }, + mockResponse + ); }); }); diff --git a/tests/unit/errors/handleError.spec.ts b/tests/unit/errors/handleError.spec.ts index d7d92e1..88384b6 100644 --- a/tests/unit/errors/handleError.spec.ts +++ b/tests/unit/errors/handleError.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, test, vi, beforeEach } from "vitest"; -import {GroutError} from "../../../src/errors/groutError"; -import {handleError} from "../../../src/errors/handleError"; -import {ErrorType} from "../../../src/errors/errorType"; +import { GroutError } from "../../../src/errors/groutError"; +import { handleError } from "../../../src/errors/handleError"; +import { ErrorType } from "../../../src/errors/errorType"; const mockJsonResponseError = vi.hoisted(() => vi.fn()); vi.mock("../../../src/jsonResponse", () => ({ @@ -26,7 +26,12 @@ describe("handleError", () => { expect(mockReq.errorDetail).toBe("test message"); expect(mockReq.errorStack).toBe(err.stack); - expect(mockJsonResponseError).toHaveBeenCalledWith(400, ErrorType.BAD_REQUEST, "test message", mockRes); + expect(mockJsonResponseError).toHaveBeenCalledWith( + 400, + ErrorType.BAD_REQUEST, + "test message", + mockRes + ); }); test("handles unexpected error", () => { @@ -36,10 +41,16 @@ describe("handleError", () => { handleError(err, mockReq, mockRes, vi.fn()); expect(mockReq.errorType).toBe(ErrorType.UNEXPECTED_ERROR); - const expectedDetail = "An unexpected error occurred. Please contact support and quote error code 123"; + const expectedDetail = + "An unexpected error occurred. Please contact support and quote error code 123"; expect(mockReq.errorDetail).toBe(expectedDetail); expect(mockReq.errorStack).toBe(err.stack); - expect(mockJsonResponseError).toHaveBeenCalledWith(500, ErrorType.UNEXPECTED_ERROR, expectedDetail, mockRes); + expect(mockJsonResponseError).toHaveBeenCalledWith( + 500, + ErrorType.UNEXPECTED_ERROR, + expectedDetail, + mockRes + ); }); }); diff --git a/tests/unit/errors/notFound.spec.ts b/tests/unit/errors/notFound.spec.ts index 433e352..ff778af 100644 --- a/tests/unit/errors/notFound.spec.ts +++ b/tests/unit/errors/notFound.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test, vi, beforeEach } from "vitest"; import notFound from "../../../src/errors/notFound"; -import {ErrorType} from "../../../src/errors/errorType"; +import { ErrorType } from "../../../src/errors/errorType"; describe("notFound", () => { test("throws expected error", () => { @@ -16,4 +16,4 @@ describe("notFound", () => { expect(err.status).toBe(404); expect(err.errorType).toBe(ErrorType.NOT_FOUND); }); -}); \ No newline at end of file +}); diff --git a/tests/unit/jsonResponse.spec.ts b/tests/unit/jsonResponse.spec.ts index e4a47eb..dbfd4a3 100644 --- a/tests/unit/jsonResponse.spec.ts +++ b/tests/unit/jsonResponse.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, test, vi, beforeEach } from "vitest"; -import {jsonResponseError, jsonResponseSuccess} from "../../src/jsonResponse"; +import { jsonResponseError, jsonResponseSuccess } from "../../src/jsonResponse"; const mockResponse = { header: vi.fn(), @@ -15,7 +15,10 @@ describe("jsonResponseSuccess", () => { test("adds expected header and content", () => { const data = { value: "test" }; jsonResponseSuccess(data, mockResponse); - expect(mockResponse.header).toHaveBeenCalledWith("Content-Type", "application/json"); + expect(mockResponse.header).toHaveBeenCalledWith( + "Content-Type", + "application/json" + ); expect(mockResponse.end).toHaveBeenCalled(); const responseObj = JSON.parse(mockResponse.end.mock.calls[0][0]); expect(responseObj).toStrictEqual({ @@ -29,17 +32,22 @@ describe("jsonResponseSuccess", () => { describe("jsonResponseError", () => { test("adds expected header and content", () => { jsonResponseError(400, "BAD_REQUEST", "test error", mockResponse); - expect(mockResponse.header).toHaveBeenCalledWith("Content-Type", "application/json"); + expect(mockResponse.header).toHaveBeenCalledWith( + "Content-Type", + "application/json" + ); expect(mockResponse.status).toHaveBeenCalledWith(400); expect(mockResponse.end).toHaveBeenCalled(); const responseObj = JSON.parse(mockResponse.end.mock.calls[0][0]); expect(responseObj).toStrictEqual({ status: "failure", - errors: [{ - error: "BAD_REQUEST", - detail: "test error" - }], + errors: [ + { + error: "BAD_REQUEST", + detail: "test error" + } + ], data: null }); }); -}); \ No newline at end of file +}); diff --git a/tests/unit/logging.spec.ts b/tests/unit/logging.spec.ts index 754e7e5..90f2269 100644 --- a/tests/unit/logging.spec.ts +++ b/tests/unit/logging.spec.ts @@ -9,7 +9,7 @@ const { mockMorganResult, mockMorgan, mockToken } = vi.hoisted(() => { return { mockMorganResult, mockMorgan, mockToken }; }); -vi.mock("morgan", () => ({ default: mockMorgan, })); +vi.mock("morgan", () => ({ default: mockMorgan })); describe("initialiseLogging", () => { test("registers morgan custom format", () => { diff --git a/tests/unit/routes.spec.ts b/tests/unit/routes.spec.ts index ac147ca..cade70a 100644 --- a/tests/unit/routes.spec.ts +++ b/tests/unit/routes.spec.ts @@ -32,6 +32,6 @@ describe("registerRoutes", () => { "/tile/:dataset/:level/:z/:x/:y", TileController.getTile ); - expect(mockRouter.use).toHaveBeenCalledWith(notFound) + expect(mockRouter.use).toHaveBeenCalledWith(notFound); }); });