From 3c260c590e96d2b8d1f76f52cb151f549c271331 Mon Sep 17 00:00:00 2001 From: Emil Janitzek Date: Sun, 5 Nov 2023 15:15:00 +0100 Subject: [PATCH 1/3] fix: request url not logged correctly --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 68332172..a6bf8037 100644 --- a/src/index.js +++ b/src/index.js @@ -983,7 +983,7 @@ module.exports = { if (req.$route && !req.$route.logging) return; if (this.settings.logRequest && this.settings.logRequest in this.logger) - this.logger[this.settings.logRequest](`=> ${req.method} ${req.url}`); + this.logger[this.settings.logRequest](`=> ${req.method} ${req.originalUrl}`); }, /** From abe26ecc81fe30d78514ab9d0b74f3203af9a085 Mon Sep 17 00:00:00 2001 From: Emil Janitzek Date: Sun, 5 Nov 2023 15:20:55 +0100 Subject: [PATCH 2/3] refactor: simplify http handler test with reusable setup functions --- test/unit/service/httpHandler.spec.js | 329 +++++++++++++------------- 1 file changed, 160 insertions(+), 169 deletions(-) diff --git a/test/unit/service/httpHandler.spec.js b/test/unit/service/httpHandler.spec.js index c9c56d07..0bb6be23 100644 --- a/test/unit/service/httpHandler.spec.js +++ b/test/unit/service/httpHandler.spec.js @@ -1,6 +1,7 @@ "use strict"; const HttpHandler = () => require("../../../src/index").methods.httpHandler; + const MockLogger = () => Object.assign({ info: jest.fn(), error: jest.fn(), @@ -8,19 +9,34 @@ const MockLogger = () => Object.assign({ debug: jest.fn(), trace: jest.fn(), }); -const MockContext = () => Object.assign({ +const MockContext = ({ action = jest.fn(), serve, settings } = {}) => Object.assign({ actions: { - rest: jest.fn(), + rest: action, }, - settings: require("../../../src/index").settings, + settings: { ...require("../../../src/index").settings, ...settings }, errorHandler: require("../../../src/index").methods.errorHandler, logger: MockLogger(), sendError: jest.fn(), send404: jest.fn(), corsHandler: jest.fn(() => false), + serve, }); -const MockRequest = () => Object.assign(jest.fn(), { headers: {} }); +const MockRequest = ({ headers = {} } = {}) => Object.assign(jest.fn(), { headers }); + +const makeFakeError = (message, code) => { + const error = new Error(message); + error.code = code; + return error; +}; + +const setup = (headers) =>{ + const handler = HttpHandler(); + const req = MockRequest(headers); + const res = jest.fn(); + const next = jest.fn(); + return { handler, req, res, next }; +}; describe("WebGateway", () => { describe("methods", () => { @@ -29,212 +45,187 @@ describe("WebGateway", () => { expect(typeof HttpHandler()).toEqual("function"); }); - it("sets $startTime of the request to the current process.hrtime()", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve()); - - return handler.bind(context)(req, res, next).then(() => { - expect(req.$startTime[0]).toBeLessThanOrEqual(process.hrtime()[0]); + it("sets $startTime of the request to the current process.hrtime()", async () => { + const { handler, req, res, next } = setup(); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce() }); + + await handler.bind(context)(req, res, next); + + expect(req.$startTime[0]).toBeLessThanOrEqual(process.hrtime()[0]); }); - it("references the context via req.$service and res.$service", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve()); - - return handler.bind(context)(req, res, next).then(() => { - expect(req.$service).toEqual(context); - expect(res.$service).toEqual(context); + it("references the context via req.$service and res.$service", async () => { + const { handler, req, res, next } = setup(); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce() }); - }); - it("references the next middleware callback via req.$next", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve()); + await handler.bind(context)(req, res, next); - return handler.bind(context)(req, res, next).then(() => { - expect(req.$next).toEqual(next); - }); + expect(req.$service).toEqual(context); }); - it("ensures that res.locals is an object if undefined", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve()); - - return handler.bind(context)(req, res, next).then(() => { - expect(res.locals).toEqual({}); + it("references the next middleware callback via req.$next", async () => { + const { handler, req, res, next } = setup(); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce() }); + + await handler.bind(context)(req, res, next); + + expect(req.$next).toEqual(next); }); - it("maintains the requestId of a \"x-request-id\" header if present", () => { - const handler = HttpHandler(); - const req = MockRequest(); - req.headers["x-request-id"] = "foobar"; - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve()); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.actions.rest.mock.calls[0]).toEqual([{ req, res }, { requestID: "foobar" }]); + it("ensures that res.locals is an object if undefined", async () => { + const { handler, req, res, next } = setup(); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce() }); + + await handler.bind(context)(req, res, next); + + expect(res.locals).toEqual({}); }); - it("maintains the requestId of a \"x-correlation-id\" header if present", () => { - const handler = HttpHandler(); - const req = MockRequest(); - req.headers["x-request-id"] = "foobar"; - req.headers["x-correlation-id"] = "barfoo"; - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve()); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.actions.rest.mock.calls[0]).toEqual([{ req, res }, { requestID: "barfoo" }]); + it("maintains the requestId of a \"x-request-id\" header if present", async () => { + const { handler, req, res, next } = setup({ headers: { "x-request-id": "foobar" } }); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce() }); - }); - it("resolves if the rest.action did resolve with an object result", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve({ foo: "bar" })); + await handler.bind(context)(req, res, next); - return handler.bind(context)(req, res, next).then(result => { - expect(context.send404.mock.calls.length).toEqual(0); + expect(context.actions.rest.mock.calls[0]).toEqual([{ req, res }, { requestID: "foobar" }]); + }); + + it("maintains the requestId of a \"x-correlation-id\" header if present", async () => { + const { handler, req, res, next } = setup({ + headers: { + "x-request-id": "foobar", + "x-correlation-id": "barfoo" + } + }); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce() }); + + await handler.bind(context)(req, res, next); + + expect(context.actions.rest.mock.calls[0]).toEqual([{ req, res }, { requestID: "barfoo" }]); }); - it("sends a 404 response if the request could not be routed and serving static assets is not configured", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.actions.rest.mockReturnValueOnce(Promise.resolve(null)); + it("resolves if the rest.action did resolve with an object result", async () => { + const { handler, req, res, next } = setup(); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce({ foo: "bar" }) + }); + + await handler.bind(context)(req, res, next); - return handler.bind(context)(req, res, next).then(result => { - expect(context.send404.mock.calls[0]).toEqual([req, res]); + expect(context.send404.mock.calls.length).toEqual(0); + }); + + it("sends a 404 response if the request could not be routed and serving static assets is not configured", async () => { + const { handler, req, res, next } = setup(); + const context = MockContext({ + action: jest.fn().mockResolvedValueOnce(null), }); + + await handler.bind(context)(req, res, next); + + expect(context.send404.mock.calls[0]).toEqual([req, res]); }); - it("resolves if the request could not be routed and instead a static asset was served", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - let context = MockContext(); - context.serve = jest.fn(); - context.actions.rest.mockReturnValueOnce(Promise.resolve(null)); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.send404.mock.calls.length).toEqual(0); - expect(context.serve.mock.calls[0][0]).toEqual(req); - expect(context.serve.mock.calls[0][1]).toEqual(res); + it("resolves if the request could not be routed and instead a static asset was served", async () => { + const { handler, req, res, next } = setup(); + let context = MockContext({ + action: jest.fn().mockResolvedValueOnce(null), + serve: jest.fn(), }); + + await handler.bind(context)(req, res, next); + + expect(context.send404.mock.calls.length).toEqual(0); + expect(context.serve.mock.calls[0][0]).toEqual(req); + expect(context.serve.mock.calls[0][1]).toEqual(res); }); - it("responds with 404 if the request could not be routed and serving a static asset encountered an error", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - let context = MockContext(); + it("responds with 404 if the request could not be routed and serving a static asset encountered an error", async () => { + const { handler, req, res, next } = setup(); + let context = MockContext({ + action: jest.fn().mockResolvedValueOnce(null), + serve: jest.fn(), + }); const error = new Error("Something went wrong while serving a static asset"); - context.serve = jest.fn(); - context.actions.rest.mockReturnValueOnce(Promise.resolve(null)); - return handler.bind(context)(req, res, next).then(() => { - context.serve.mock.calls[0][2](error); - expect(context.send404.mock.calls[0]).toEqual([req, res]); - expect(context.logger.debug.mock.calls[0]).toEqual([error]); - }); + await handler.bind(context)(req, res, next); + context.serve.mock.calls[0][2](error); + + expect(context.send404.mock.calls[0]).toEqual([req, res]); + expect(context.logger.debug.mock.calls[0]).toEqual([error]); }); - it("logs and responds with an error if the rest action rejects", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - let error = new Error("Something went wrong while invoking the rest action"); - error.code = 503; - context.actions.rest.mockReturnValueOnce(Promise.reject(error)); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); - expect(context.logger.error.mock.calls[0]).toEqual([" Request error!", error.name, ":", error.message, "\n", error.stack, "\nData:", error.data]); + it("logs and responds with an error if the rest action rejects", async () => { + const { handler, req, res, next } = setup(); + const error = makeFakeError("Something went wrong while invoking the rest action", 503); + const context = MockContext({ + action: jest.fn().mockRejectedValueOnce(error) }); + + await handler.bind(context)(req, res, next); + + expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); + expect(context.logger.error.mock.calls[0]).toEqual([" Request error!", error.name, ":", error.message, "\n", error.stack, "\nData:", error.data]); }); - it("responds with an error but does not log the error if the rest action rejects, the error code is 400 and settings.log4XXResponses is false", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.settings.log4XXResponses = false; - let error = new Error("Something went wrong while invoking the rest action"); - error.code = 400; - context.actions.rest.mockReturnValueOnce(Promise.reject(error)); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); - expect(context.logger.error.mock.calls.length).toEqual(0); + it("responds with an error but does not log the error if the rest action rejects, the error code is 400 and settings.log4XXResponses is false", async () => { + const { handler, req, res, next } = setup(); + const error = makeFakeError("Something went wrong while invoking the rest action", 400); + const context = MockContext({ + action: jest.fn().mockRejectedValueOnce(error), + settings: { + log4XXResponses: false + } }); + + await handler.bind(context)(req, res, next); + + expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); + expect(context.logger.error.mock.calls.length).toEqual(0); }); - it("logs and responds with an error if the rest action rejects, the error code is 399 and settings.log4XXResponses is false", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.settings.log4XXResponses = false; - let error = new Error("Something went wrong while invoking the rest action"); - error.code = 399; - context.actions.rest.mockReturnValueOnce(Promise.reject(error)); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); - expect(context.logger.error.mock.calls.length).toEqual(1); + it("logs and responds with an error if the rest action rejects, the error code is 399 and settings.log4XXResponses is false", async () => { + const { handler, req, res, next } = setup(); + const error = makeFakeError("Something went wrong while invoking the rest action", 399); + const context = MockContext({ + action: jest.fn().mockRejectedValueOnce(error), + settings: { + log4XXResponses: false + } }); + + await handler.bind(context)(req, res, next); + + expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); + expect(context.logger.error.mock.calls.length).toEqual(1); }); - it("logs and responds with an error if the rest action rejects, the error code is 500 and settings.log4XXResponses is false", () => { - const handler = HttpHandler(); - const req = MockRequest(); - const res = jest.fn(); - const next = jest.fn(); - const context = MockContext(); - context.settings.log4XXResponses = false; - let error = new Error("Something went wrong while invoking the rest action"); - error.code = 500; - context.actions.rest.mockReturnValueOnce(Promise.reject(error)); - - return handler.bind(context)(req, res, next).then(() => { - expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); - expect(context.logger.error.mock.calls.length).toEqual(1); + it("logs and responds with an error if the rest action rejects, the error code is 500 and settings.log4XXResponses is false", async () => { + const { handler, req, res, next } = setup(); + const error = makeFakeError("Something went wrong while invoking the rest action", 500); + const context = MockContext({ + action: jest.fn().mockRejectedValueOnce(error), + settings: { + log4XXResponses: false + } }); + + await handler.bind(context)(req, res, next); + + expect(context.sendError.mock.calls[0]).toEqual([req, res, error]); + expect(context.logger.error.mock.calls.length).toEqual(1); }); }); }); From 9820ae66ef998da11740dbf33d2ab27ea4131611 Mon Sep 17 00:00:00 2001 From: Icebob Date: Sun, 12 Nov 2023 18:03:11 +0100 Subject: [PATCH 3/3] update node versions --- .github/workflows/nodejs.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 91d2b221..ffc366a8 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -8,15 +8,15 @@ jobs: strategy: matrix: - node-version: [10.x, 12.x, 14.x, 15.x] + node-version: [10.x, 12.x, 14.x, 16.x, 18.x, 20.x] steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v2 with: node-version: ${{ matrix.node-version }} - - uses: actions/cache@v1 + - uses: actions/cache@v2 with: path: node_modules key: ${{ matrix.node-version }}-node-${{ hashFiles('**/package-lock.json') }}