Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes the onUnhandledRequest middleware option #740

Merged
merged 5 commits into from
Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -597,32 +597,6 @@ createServer(middleware).listen(3000);

Used for internal logging. Defaults to [`console`](https://developer.mozilla.org/en-US/docs/Web/API/console) with `debug` and `info` doing nothing.

</td>
</tr>
<tr>
<td>
<code>onUnhandledRequest</code>
<em>
function
</em>
</td>
<td>

Defaults to

```js
function onUnhandledRequest(request, response) {
response.writeHead(400, {
"content-type": "application/json",
});
response.end(
JSON.stringify({
error: error.message,
})
);
}
```

</td>
</tr>
<tbody>
Expand Down
3 changes: 0 additions & 3 deletions src/middleware/node/index.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
import { createLogger } from "../../createLogger";
import { Webhooks } from "../../index";
import { middleware } from "./middleware";
import { onUnhandledRequestDefault } from "./on-unhandled-request-default";
import { MiddlewareOptions } from "./types";

export function createNodeMiddleware(
webhooks: Webhooks,
{
path = "/api/github/webhooks",
onUnhandledRequest = onUnhandledRequestDefault,
log = createLogger(),
}: MiddlewareOptions = {}
) {
return middleware.bind(null, webhooks, {
path,
onUnhandledRequest,
log,
} as Required<MiddlewareOptions>);
}
28 changes: 15 additions & 13 deletions src/middleware/node/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ import { WebhookEventHandlerError } from "../../types";
import { MiddlewareOptions } from "./types";
import { getMissingHeaders } from "./get-missing-headers";
import { getPayload } from "./get-payload";
import { onUnhandledRequestDefault } from "./on-unhandled-request-default";

export async function middleware(
webhooks: Webhooks,
options: Required<MiddlewareOptions>,
request: IncomingMessage,
response: ServerResponse,
next?: Function
) {
): Promise<boolean> {
let pathname: string;
try {
pathname = new URL(request.url as string, "http://localhost").pathname;
Expand All @@ -31,17 +32,15 @@ export async function middleware(
error: `Request URL could not be parsed: ${request.url}`,
})
);
return;
return true;
}

const isUnknownRoute = request.method !== "POST" || pathname !== options.path;
const isExpressMiddleware = typeof next === "function";
if (isUnknownRoute) {
if (isExpressMiddleware) {
return next!();
} else {
return options.onUnhandledRequest(request, response);
}
if (pathname !== options.path) {
next?.();
return false;
} else if (request.method !== "POST") {
onUnhandledRequestDefault(request, response);
return true;
}

const missingHeaders = getMissingHeaders(request).join(", ");
Expand All @@ -56,7 +55,7 @@ export async function middleware(
})
);

return;
return true;
}

const eventName = request.headers["x-github-event"] as WebhookEventName;
Expand Down Expand Up @@ -85,13 +84,14 @@ export async function middleware(
});
clearTimeout(timeout);

if (didTimeout) return;
if (didTimeout) return true;

response.end("ok\n");
return true;
} catch (error) {
clearTimeout(timeout);

if (didTimeout) return;
if (didTimeout) return true;

const err = Array.from(error as WebhookEventHandlerError)[0];
const errorMessage = err.message
Expand All @@ -106,5 +106,7 @@ export async function middleware(
error: errorMessage,
})
);

return true;
}
}
4 changes: 0 additions & 4 deletions src/middleware/node/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,4 @@ import { Logger } from "../../createLogger";
export type MiddlewareOptions = {
path?: string;
log?: Logger;
onUnhandledRequest?: (
baoshan marked this conversation as resolved.
Show resolved Hide resolved
request: IncomingMessage,
response: ServerResponse
) => void;
};
43 changes: 20 additions & 23 deletions test/integration/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { sign } from "@octokit/webhooks-methods";
// import without types
const express = require("express");

import { Webhooks, createNodeMiddleware } from "../../src";
import { createNodeMiddleware, Webhooks } from "../../src";

const pushEventPayload = readFileSync(
"test/fixtures/push-payload.json",
Expand Down Expand Up @@ -168,39 +168,36 @@ describe("createNodeMiddleware(webhooks)", () => {
server.close();
});

test("custom non-found handler", async () => {
test("handle unhandled requests", async () => {
const webhooks = new Webhooks({
secret: "mySecret",
});

const server = createServer(
createNodeMiddleware(webhooks, {
onUnhandledRequest(_request, response) {
response.writeHead(404);
response.end("nope");
},
})
).listen();
const middleware = createNodeMiddleware(webhooks, {});
const server = createServer(async (req, res) => {
if (!(await middleware(req, res))) {
res.writeHead(404, { "Content-Type": "text/plain" });
res.write("nope.");
res.end();
}
}).listen();

// @ts-expect-error complains about { port } although it's included in returned AddressInfo interface
const { port } = server.address();

const response = await fetch(
`http://localhost:${port}/api/github/webhooks`,
{
method: "PUT",
headers: {
"X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000",
"X-GitHub-Event": "push",
"X-Hub-Signature-256": signatureSha256,
},
body: "invalid",
}
);
const response = await fetch(`http://localhost:${port}/foo`, {
method: "PUT",
headers: {
"X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000",
"X-GitHub-Event": "push",
"X-Hub-Signature-256": signatureSha256,
},
body: "invalid",
});

expect(response.status).toEqual(404);

await expect(response.text()).resolves.toEqual("nope");
await expect(response.text()).resolves.toEqual("nope.");

server.close();
});
Expand Down