feat: add forwardError option to enable error forwarding to next middleware

src/index.js

@@ -125,6 +125,7 @@ const noop = () => {};
  * @property {boolean=} lastModified options to generate last modified header
  * @property {(boolean | number | string | { maxAge?: number, immutable?: boolean })=} cacheControl options to generate cache headers
  * @property {boolean=} cacheImmutable is cache immutable
+ * @property {boolean=} forwardError forward error to next middleware
  */
 
 /**

src/middleware.js

@@ -161,8 +161,6 @@ function wrapper(context) {
     }
 
     const acceptedMethods = context.options.methods || ["GET", "HEAD"];
-    // TODO do we need an option here?
-    const forwardError = false;
 
     initState(res);
 
@@ -180,13 +178,22 @@ function wrapper(context) {
      * @returns {Promise<void>}
      */
     async function sendError(message, status, options) {
-      if (forwardError) {
+      if (context.options.forwardError) {
+        if (!getHeadersSent(res)) {
+          const headers = getResponseHeaders(res);
+
+          for (let i = 0; i < headers.length; i++) {
+            removeResponseHeader(res, headers[i]);
+          }
+        }
+
         const error =
           /** @type {Error & { statusCode: number }} */
           (new Error(message));
         error.statusCode = status;
 
         await goNext(error);
+        return;
       }
 
       const escapeHtml = getEscapeHtml();

src/options.json

@@ -172,6 +172,11 @@
       "description": "Enable or disable setting `Cache-Control: public, max-age=31536000, immutable` response header for immutable assets (i.e. asset with a hash in file name like `image.a4c12bde.jpg`).",
       "link": "https://github.com/webpack/webpack-dev-middleware#cacheimmutable",
       "type": "boolean"
+    },
+    "forwardError": {
+      "description": "Enable or disable forwarding errors to next middleware.",
+      "link": "https://github.com/webpack/webpack-dev-middleware#forwarderrors",
+      "type": "boolean"
     }
   },
   "additionalProperties": false

test/middleware.test.js

@@ -146,7 +146,6 @@ async function frameworkFactory(
           app.use(item);
         }
       }
-
       return [server, req, instance.devMiddleware];
     }
     default: {
@@ -3610,6 +3609,224 @@ describe.each([
         });
       });
 
+      (name === "koa" || name === "hono" ? describe.skip : describe)(
+        "should call the next middleware for finished or errored requests when forwardError is enabled",
+        () => {
+          let compiler;
+
+          const outputPath = path.resolve(
+            __dirname,
+            "./outputs/basic-test-errors-headers-sent",
+          );
+
+          let nextWasCalled = false;
+
+          beforeAll(async () => {
+            compiler = getCompiler({
+              ...webpackConfig,
+              output: {
+                filename: "bundle.js",
+                path: outputPath,
+              },
+            });
+
+            [server, req, instance] = await frameworkFactory(
+              name,
+              framework,
+              compiler,
+              {
+                etag: "weak",
+                lastModified: true,
+                forwardError: 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 {
+                    middlewares.push((_error, _req, res, _next) => {
+                      nextWasCalled = true;
+                      res.statusCode = 500;
+                      res.end("error");
+                    });
+                  }
+
+                  return middlewares;
+                },
+              },
+            );
+
+            instance.context.outputFileSystem.mkdirSync(outputPath, {
+              recursive: true,
+            });
+            instance.context.outputFileSystem.writeFileSync(
+              path.resolve(outputPath, "index.html"),
+              "HTML",
+            );
+            instance.context.outputFileSystem.writeFileSync(
+              path.resolve(outputPath, "image.svg"),
+              "svg image",
+            );
+            instance.context.outputFileSystem.writeFileSync(
+              path.resolve(outputPath, "file.text"),
+              "text",
+            );
+
+            const originalMethod =
+              instance.context.outputFileSystem.createReadStream;
+
+            instance.context.outputFileSystem.createReadStream =
+              function createReadStream(...args) {
+                if (args[0].endsWith("image.svg")) {
+                  const brokenStream = new this.ReadStream(...args);
+
+                  brokenStream._read = function _read() {
+                    const error = new Error("test");
+                    error.code = "ENAMETOOLONG";
+                    this.emit("error", error);
+                    this.end();
+                    this.destroy();
+                  };
+
+                  return brokenStream;
+                }
+
+                return originalMethod(...args);
+              };
+          });
+
+          beforeEach(() => {
+            nextWasCalled = false;
+          });
+
+          afterAll(async () => {
+            await close(server, instance);
+          });
+
+          it("should work with piping stream", async () => {
+            const response1 = await req.get("/file.text");
+
+            expect(response1.statusCode).toBe(200);
+            expect(nextWasCalled).toBe(false);
+          });
+
+          it('should return the "500" code for requests above root', async () => {
+            const response = await req.get(
+              "/public/..%2f../middleware.test.js",
+            );
+
+            expect(response.statusCode).toBe(500);
+            if (name !== "hapi") {
+              expect(response.text).toBe("error");
+              expect(nextWasCalled).toBe(true);
+            } else {
+              expect(nextWasCalled).toBe(false);
+            }
+          });
+
+          it('should return the "500" code for the "GET" request to the bundle file with etag and wrong "if-match" header', 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-match", "test");
+
+            expect(response2.statusCode).toBe(500);
+            if (name !== "hapi") {
+              expect(response2.text).toBe("error");
+              expect(nextWasCalled).toBe(true);
+            } else {
+              expect(nextWasCalled).toBe(false);
+            }
+          });
+
+          it('should return the "500" code for the "GET" request with the invalid range header', async () => {
+            const response = await req
+              .get("/file.text")
+              .set("Range", "bytes=9999999-");
+
+            expect(response.statusCode).toBe(500);
+            if (name !== "hapi") {
+              expect(response.text).toBe("error");
+              expect(nextWasCalled).toBe(true);
+            } else {
+              expect(nextWasCalled).toBe(false);
+            }
+          });
+
+          it('should return the "500" code for the "GET" request to the "image.svg" file when it throws a reading error', async () => {
+            const response = await req.get("/image.svg");
+
+            expect(response.statusCode).toBe(500);
+            if (name !== "hapi") {
+              expect(response.text).toBe("error");
+              expect(nextWasCalled).toBe(true);
+            } else {
+              expect(nextWasCalled).toBe(false);
+            }
+          });
+
+          it('should return the "200" code for the "HEAD" request to the bundle file', async () => {
+            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;
 

types/index.d.ts

@@ -94,6 +94,7 @@ export = wdm;
  * @property {boolean=} lastModified options to generate last modified header
  * @property {(boolean | number | string | { maxAge?: number, immutable?: boolean })=} cacheControl options to generate cache headers
  * @property {boolean=} cacheImmutable is cache immutable
+ * @property {boolean=} forwardError forward error to next middleware
  */
 /**
  * @template {IncomingMessage} [RequestInternal=IncomingMessage]
@@ -449,6 +450,10 @@ type Options<
    * is cache immutable
    */
   cacheImmutable?: boolean | undefined;
+  /**
+   * forward error to next middleware
+   */
+  forwardError?: boolean | undefined;
 };
 type Middleware<
   RequestInternal extends IncomingMessage = import("http").IncomingMessage,