From 7d453557eeedea90e4dac4ae0420171230789e6e Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Thu, 4 Sep 2025 20:35:24 +0300 Subject: [PATCH] refactor: simplify code and improve tests --- src/middleware.js | 18 +-- test/middleware.test.js | 339 ++++++++++++++++------------------------ 2 files changed, 139 insertions(+), 218 deletions(-) diff --git a/src/middleware.js b/src/middleware.js index 02cfd497d..ed858d6f9 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -172,16 +172,16 @@ function wrapper(context) { } /** - * @param {NodeJS.ErrnoException} err ann error + * @param {string} message an error message * @param {number} status status * @param {Partial>=} options options * @returns {Promise} */ - async function sendError(err, status, options) { + async function sendError(message, status, options) { if (forwardError) { const error = /** @type {Error & { statusCode: number }} */ - (new Error(err.message)); + (new Error(message)); error.statusCode = status; await goNext(error); @@ -252,12 +252,12 @@ function wrapper(context) { case "ENAMETOOLONG": case "ENOENT": case "ENOTDIR": - await sendError(error, 404, { + await sendError(error.message, 404, { modifyResponseData: context.options.modifyResponseData, }); break; default: - await sendError(error, 500, { + await sendError(error.message, 500, { modifyResponseData: context.options.modifyResponseData, }); break; @@ -512,9 +512,7 @@ function wrapper(context) { } await sendError( - extra.errorCode === 400 - ? new Error("Bad Request") - : new Error("Forbidden"), + extra.errorCode === 400 ? "Bad Request" : "Forbidden", extra.errorCode, { modifyResponseData: context.options.modifyResponseData, @@ -708,7 +706,7 @@ function wrapper(context) { // Conditional GET support if (isConditionalGET()) { if (isPreconditionFailure()) { - await sendError(new Error("Precondition Failed"), 412, { + await sendError("Precondition Failed", 412, { modifyResponseData: context.options.modifyResponseData, }); return; @@ -760,7 +758,7 @@ function wrapper(context) { getValueContentRangeHeader("bytes", size), ); - await sendError(new Error("Range Not Satisfiable"), 416, { + await sendError("Range Not Satisfiable", 416, { headers: { "Content-Range": getResponseHeader(res, "Content-Range"), }, diff --git a/test/middleware.test.js b/test/middleware.test.js index 101bdbbbd..f6b59b03a 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -3318,23 +3318,21 @@ describe.each([ if (name === "hapi") { // There's no such thing as "the next route handler" in hapi. One request is matched to one or no route handlers. } else if (name === "koa") { - middlewares.push(async (ctx, next) => { + middlewares.unshift(async (ctx, next) => { ctx.url = "/index.html"; await next(); }); - middlewares.push(middleware.koaWrapper(compiler, {})); } else if (name === "hono") { // Hono doesn't have this problem due design } else { - middlewares.push({ + middlewares.unshift({ route: "/", fn: (oldReq, res, next) => { oldReq.url = "/index.html"; next(); }, }); - middlewares.push(middleware(compiler, {})); } return middlewares; @@ -3400,20 +3398,19 @@ describe.each([ compiler, { etag: "weak", + lastModified: true, }, { setupMiddlewares: (middlewares) => { if (name === "hapi") { // There's no such thing as "the next route handler" in hapi. One request is matched to one or no route handlers. } else if (name === "koa") { - middlewares.push(middleware.koaWrapper(compiler, {})); middlewares.push(async () => { nextWasCalled = true; }); } else if (name === "hono") { // Hono doesn't have this problem due design } else { - middlewares.push(middleware(compiler, {})); middlewares.push(() => { nextWasCalled = true; }); @@ -3435,6 +3432,10 @@ describe.each([ path.resolve(outputPath, "image.svg"), "svg image", ); + instance.context.outputFileSystem.writeFileSync( + path.resolve(outputPath, "file.text"), + "text", + ); const originalMethod = instance.context.outputFileSystem.createReadStream; @@ -3464,7 +3465,7 @@ describe.each([ }); it("should work with piping stream", async () => { - const response1 = await req.get("/"); + const response1 = await req.get("/file.text"); expect(response1.statusCode).toBe(200); expect(nextWasCalled).toBe(false); @@ -3491,20 +3492,22 @@ describe.each([ }); it('should return the "412" code for the "GET" request to the bundle file with etag and wrong "if-match" header', async () => { - const response1 = await req.get("/"); + const response1 = await req.get("/file.text"); expect(response1.statusCode).toBe(200); expect(response1.headers.etag).toBeDefined(); expect(response1.headers.etag.startsWith("W/")).toBe(true); - const response2 = await req.get("/").set("if-match", "test"); + const response2 = await req.get("/file.text").set("if-match", "test"); expect(response2.statusCode).toBe(412); expect(nextWasCalled).toBe(false); }); it('should return the "416" code for the "GET" request with the invalid range header', async () => { - const response = await req.get("/").set("Range", "bytes=9999999-"); + const response = await req + .get("/file.text") + .set("Range", "bytes=9999999-"); expect(response.statusCode).toBe(416); expect(response.headers["content-type"]).toBe( @@ -3548,12 +3551,129 @@ describe.each([ }); it('should return the "200" code for the "HEAD" request to the bundle file', async () => { - const response = await req.head("/"); + const response = await req.head("/file.text"); expect(response.statusCode).toBe(200); expect(response.text).toBeUndefined(); expect(nextWasCalled).toBe(false); }); + + it('should return the "304" code for the "GET" request to the bundle file with etag and "if-none-match"', async () => { + const response1 = await req.get("/file.text"); + + expect(response1.statusCode).toBe(200); + expect(response1.headers.etag).toBeDefined(); + expect(response1.headers.etag.startsWith("W/")).toBe(true); + + const response2 = await req + .get("/file.text") + .set("if-none-match", response1.headers.etag); + + expect(response2.statusCode).toBe(304); + expect(response2.headers.etag).toBeDefined(); + expect(response2.headers.etag.startsWith("W/")).toBe(true); + + const response3 = await req + .get("/file.text") + .set("if-none-match", response1.headers.etag); + + expect(response3.statusCode).toBe(304); + expect(response3.headers.etag).toBeDefined(); + expect(response3.headers.etag.startsWith("W/")).toBe(true); + expect(nextWasCalled).toBe(false); + }); + + it('should return the "304" code for the "GET" request to the bundle file with lastModified and "if-modified-since" header', async () => { + const response1 = await req.get("/file.text"); + + expect(response1.statusCode).toBe(200); + expect(response1.headers["last-modified"]).toBeDefined(); + + const response2 = await req + .get("/file.text") + .set("if-modified-since", response1.headers["last-modified"]); + + expect(response2.statusCode).toBe(304); + expect(response2.headers["last-modified"]).toBeDefined(); + + const response3 = await req + .get("/file.text") + .set("if-modified-since", response2.headers["last-modified"]); + + expect(response3.statusCode).toBe(304); + expect(response3.headers["last-modified"]).toBeDefined(); + expect(nextWasCalled).toBe(false); + }); + }); + + describe("should fallthrough for not found files", () => { + let compiler; + + const outputPath = path.resolve( + __dirname, + "./outputs/basic-not-found-files", + ); + + let nextWasCalled = false; + + beforeAll(async () => { + compiler = getCompiler({ + ...webpackConfig, + output: { + filename: "bundle.js", + path: outputPath, + }, + }); + + [server, req, instance] = await frameworkFactory( + name, + framework, + compiler, + {}, + { + setupMiddlewares: (middlewares) => { + if (name === "hapi") { + nextWasCalled = true; + } else if (name === "koa") { + middlewares.push(async (ctx, next) => { + nextWasCalled = true; + + await next(); + }); + } else if (name === "hono") { + middlewares.push(async (c, next) => { + await next(); + + nextWasCalled = true; + }); + } else { + middlewares.push((req, res, next) => { + nextWasCalled = true; + + next(); + }); + } + + return middlewares; + }, + }, + ); + + instance.context.outputFileSystem.mkdirSync(outputPath, { + recursive: true, + }); + }); + + afterAll(async () => { + await close(server, instance); + }); + + it("should work", async () => { + const response = await req.get("/not-found.js"); + + expect(response.statusCode).toBe(404); + expect(nextWasCalled).toBe(true); + }); }); }); @@ -5530,106 +5650,6 @@ describe.each([ }); }); - describe("should work and generate etag with other middlewares", () => { - beforeAll(async () => { - const compiler = getCompiler(webpackConfig); - - let isFirstRequest = true; - - [server, req, instance] = await frameworkFactory( - name, - framework, - compiler, - { - etag: "weak", - }, - { - setupMiddlewares: (middlewares) => { - if (name === "koa") { - middlewares.push(async (ctx, next) => { - await next(); - - if (!isFirstRequest) { - ctx.status = 500; - ctx.body = "shouldn't get there"; - } - - isFirstRequest = false; - }); - } else if (name === "hapi") { - middlewares.push({ - plugin: { - name: "myPlugin", - version: "1.0.0", - register(innerServer) { - innerServer.ext("onRequest", () => { - if (!isFirstRequest) { - throw new Error("shouldn't get there"); - } - - isFirstRequest = false; - }); - }, - }, - }); - } else if (name === "hono") { - middlewares.push(async (c, next) => { - await next(); - - if (!isFirstRequest) { - throw new Error("shouldn't get there"); - } - - isFirstRequest = false; - }); - } else { - middlewares.push(async (req, res, next) => { - if (!isFirstRequest) { - next(new Error("shouldn't get there")); - return; - } - - isFirstRequest = false; - - next(); - }); - } - - return middlewares; - }, - }, - ); - }); - - afterAll(async () => { - await close(server, instance); - }); - - it('should return the "304" code for the "GET" request to the bundle file with etag and "if-none-match" header with additional middlewares', async () => { - const response1 = await req.get("/bundle.js"); - - expect(response1.statusCode).toBe(200); - expect(response1.headers.etag).toBeDefined(); - expect(response1.headers.etag.startsWith("W/")).toBe(true); - - const response2 = await req - .get("/bundle.js") - .set("if-none-match", response1.headers.etag); - - expect(response2.statusCode).toBe(304); - expect(response2.headers.etag).toBeDefined(); - expect(response2.headers.etag.startsWith("W/")).toBe(true); - - const response3 = await req - .get("/bundle.js") - .set("if-none-match", response1.headers.etag); - - expect(response3.statusCode).toBe(304); - expect(response3.headers.etag).toBeDefined(); - expect(response3.headers.etag.startsWith("W/")).toBe(true); - }); - }); - describe("should work and generate strong etag without createReadStream", () => { beforeEach(async () => { const compiler = getCompiler(webpackConfig); @@ -5869,103 +5889,6 @@ describe.each([ expect(response2.headers.etag).toBeDefined(); }); }); - - describe("should work and generate etag with other middlewares", () => { - beforeAll(async () => { - const compiler = getCompiler(webpackConfig); - - let isFirstRequest = true; - - [server, req, instance] = await frameworkFactory( - name, - framework, - compiler, - { - lastModified: true, - }, - { - setupMiddlewares: (middlewares) => { - if (name === "koa") { - middlewares.push(async (ctx, next) => { - await next(); - - if (!isFirstRequest) { - ctx.status = 500; - ctx.body = "shouldn't get there"; - } - - isFirstRequest = false; - }); - } else if (name === "hapi") { - middlewares.push({ - plugin: { - name: "myPlugin", - version: "1.0.0", - register(innerServer) { - innerServer.ext("onRequest", () => { - if (!isFirstRequest) { - throw new Error("shouldn't get there"); - } - - isFirstRequest = false; - }); - }, - }, - }); - } else if (name === "hono") { - middlewares.push(async (c, next) => { - await next(); - - if (!isFirstRequest) { - throw new Error("shouldn't get there"); - } - - isFirstRequest = false; - }); - } else { - middlewares.push(async (req, res, next) => { - if (!isFirstRequest) { - next(new Error("shouldn't get there")); - return; - } - - isFirstRequest = false; - - next(); - }); - } - - return middlewares; - }, - }, - ); - }); - - afterAll(async () => { - await close(server, instance); - }); - - it('should return the "304" code for the "GET" request to the bundle file with lastModified and "if-modified-since" header with additional middlewares', async () => { - const response1 = await req.get("/bundle.js"); - - expect(response1.statusCode).toBe(200); - expect(response1.headers["last-modified"]).toBeDefined(); - - const response2 = await req - .get("/bundle.js") - .set("if-modified-since", response1.headers["last-modified"]); - - expect(response2.statusCode).toBe(304); - expect(response2.headers["last-modified"]).toBeDefined(); - - const response3 = await req - .get("/bundle.js") - .set("if-modified-since", response2.headers["last-modified"]); - - expect(response3.statusCode).toBe(304); - expect(response3.headers["last-modified"]).toBeDefined(); - }); - }); }); describe("cacheControl", () => {