Skip to content

Commit

Permalink
error endpoint test, lint, grey country border for test page
Browse files Browse the repository at this point in the history
  • Loading branch information
EmmaLRussell committed Dec 17, 2024
1 parent 2d5c3c0 commit ab485a9
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/errors/asyncControllerHandler.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions src/errors/handleError.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
4 changes: 3 additions & 1 deletion src/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test.html
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ <h1>GROUT</h1>
style: {
weight: 1,
fill: false,
color: "#000000"
color: "#777"
},
...options
};
Expand Down
46 changes: 30 additions & 16 deletions tests/integration/errors.spec.ts
Original file line number Diff line number Diff line change
@@ -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) => {
Expand All @@ -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 }]
});
};

Expand All @@ -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}$/
);
});
});
});
5 changes: 4 additions & 1 deletion tests/unit/controllers/indexController.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});
});
23 changes: 17 additions & 6 deletions tests/unit/errors/handleError.spec.ts
Original file line number Diff line number Diff line change
@@ -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", () => ({
Expand All @@ -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", () => {
Expand All @@ -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
);
});
});
4 changes: 2 additions & 2 deletions tests/unit/errors/notFound.spec.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -16,4 +16,4 @@ describe("notFound", () => {
expect(err.status).toBe(404);
expect(err.errorType).toBe(ErrorType.NOT_FOUND);
});
});
});
24 changes: 16 additions & 8 deletions tests/unit/jsonResponse.spec.ts
Original file line number Diff line number Diff line change
@@ -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(),
Expand All @@ -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({
Expand All @@ -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
});
});
});
});
2 changes: 1 addition & 1 deletion tests/unit/logging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ describe("registerRoutes", () => {
"/tile/:dataset/:level/:z/:x/:y",
TileController.getTile
);
expect(mockRouter.use).toHaveBeenCalledWith(notFound)
expect(mockRouter.use).toHaveBeenCalledWith(notFound);
});
});

0 comments on commit ab485a9

Please sign in to comment.