Repository: octokit/webhooks.js
Pull request #534 (2021-04-15 20:08:18)

fix: pass on to the next middleware in case of express
As I explained [here](https://github.com/octokit/webhooks.js/pull/509#issuecomment-811379891), it is now required to always install this middleware to the end of the chain, otherwise it rejects all requests it cannot handle.

honnix: @gr2m PTAL and help me understand whether this is a valid change. Thanks a lot.
honnix: Sorry I couldn't figure out how to add a correct label. I guess this is a `bug`.

Patch:
diff --git a/src/middleware/node/middleware.ts b/src/middleware/node/middleware.ts
index f6dab98..25219b3 100644
--- a/src/middleware/node/middleware.ts
+++ b/src/middleware/node/middleware.ts
@@ -23,8 +23,12 @@ export async function middleware(
 
   const isUnknownRoute = request.method !== "POST" || pathname !== options.path;
   const isExpressMiddleware = typeof next === "function";
-  if (!isExpressMiddleware && isUnknownRoute) {
-    return options.onUnhandledRequest(request, response);
+  if (isUnknownRoute) {
+    if (isExpressMiddleware) {
+      return next!();
+    } else {
+      return options.onUnhandledRequest(request, response);
+    }
   }
 
   const missingHeaders = getMissingHeaders(request).join(", ");


Test patch:
diff --git a/test/integration/node-middleware.test.ts b/test/integration/node-middleware.test.ts
index 189d1f7..aed4972 100644
--- a/test/integration/node-middleware.test.ts
+++ b/test/integration/node-middleware.test.ts
@@ -331,13 +331,102 @@ describe("createNodeMiddleware(webhooks)", () => {
     expect(response.status).toEqual(202);
   });
 
-  test("express middleware 404", async () => {
+  test("express middleware no mount path 404", async () => {
     const app = express();
     const webhooks = new Webhooks({
       secret: "mySecret",
     });
 
-    app.post("/test", createNodeMiddleware(webhooks));
+    app.use(createNodeMiddleware(webhooks));
+    app.all("*", (_request: any, response: any) =>
+      response.status(404).send("Dafuq")
+    );
+
+    const server = app.listen();
+
+    const { port } = server.address();
+
+    const response = await fetch(`http://localhost:${port}/test`, {
+      method: "POST",
+      body: JSON.stringify(pushEventPayload),
+    });
+
+    await expect(response.text()).resolves.toBe("Dafuq");
+    expect(response.status).toEqual(404);
+
+    server.close();
+  });
+
+  test("express middleware no mount path no next", async () => {
+    const app = express();
+    const webhooks = new Webhooks({
+      secret: "mySecret",
+    });
+
+    app.all("/foo", (_request: any, response: any) => response.end("ok\n"));
+    app.use(createNodeMiddleware(webhooks));
+
+    const server = app.listen();
+
+    const { port } = server.address();
+
+    const response = await fetch(`http://localhost:${port}/test`, {
+      method: "POST",
+      body: JSON.stringify(pushEventPayload),
+    });
+
+    await expect(response.text()).resolves.toContain("Cannot POST /test");
+    expect(response.status).toEqual(404);
+
+    const responseForFoo = await fetch(`http://localhost:${port}/foo`, {
+      method: "POST",
+      body: JSON.stringify(pushEventPayload),
+    });
+
+    await expect(responseForFoo.text()).resolves.toContain("ok\n");
+    expect(responseForFoo.status).toEqual(200);
+
+    server.close();
+  });
+
+  test("express middleware no mount path with options.path", async () => {
+    const app = express();
+    const webhooks = new Webhooks({
+      secret: "mySecret",
+    });
+
+    app.use(createNodeMiddleware(webhooks, { path: "/test" }));
+    app.all("*", (_request: any, response: any) =>
+      response.status(404).send("Dafuq")
+    );
+
+    const server = app.listen();
+
+    const { port } = server.address();
+
+    const response = await fetch(`http://localhost:${port}/test`, {
+      method: "POST",
+      headers: {
+        "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000",
+        "X-GitHub-Event": "push",
+        "X-Hub-Signature-256": signatureSha256,
+      },
+      body: JSON.stringify(pushEventPayload),
+    });
+
+    await expect(response.text()).resolves.toBe("ok\n");
+    expect(response.status).toEqual(200);
+
+    server.close();
+  });
+
+  test("express middleware with mount path with options.path", async () => {
+    const app = express();
+    const webhooks = new Webhooks({
+      secret: "mySecret",
+    });
+
+    app.post("/test", createNodeMiddleware(webhooks, { path: "/test" }));
     app.all("*", (_request: any, response: any) =>
       response.status(404).send("Dafuq")
     );