refactor!: remove onUnhandledRequest middleware option

octokit · Sep 9, 2022 · 882f7ac · 882f7ac
1 parent e6baaf1
commit 882f7ac
Show file tree

Hide file tree

Showing 13 changed files with 184 additions and 378 deletions.
diff --git a/README.md b/README.md
@@ -937,31 +937,6 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/
 
 </td>
     </tr>
-    <tr>
-      <th>
-        <code>options.onUnhandledRequest</code> __deprecated__
-      </th>
-      <th>
-        <code>function</code>
-      </th>
-      <td>
-
-Defaults to
-
-```js
-function onUnhandledRequest(request, response) {
-  response.writeHead(404, {
-    "content-type": "application/json",
-  });
-  response.end(
-    JSON.stringify({
-      error: `Unknown route: ${request.method} ${request.url}`,
-    })
-  );
-}
-```
-
-</td></tr>
   </tbody>
 </table>
 
@@ -1025,31 +1000,6 @@ addEventListener("fetch", (event) => {
 
 All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"`
 
-</td>
-    </tr>
-    <tr>
-      <th>
-        <code>options.onUnhandledRequest</code> __deprecated__
-      </th>
-      <th>
-        <code>function</code>
-      </th>
-      <td>Defaults to
-
-```js
-function onUnhandledRequest(request) {
-  return new Response(
-    JSON.stringify({
-      error: `Unknown route: ${request.method} ${request.url}`,
-    }),
-    {
-      status: 404,
-      headers: { "content-type": "application/json" },
-    }
-  );
-}
-```
-
 </td>
     </tr>
   </tbody>
@@ -1115,31 +1065,6 @@ export const handler = createAWSLambdaAPIGatewayV2Handler(app, {
 
 All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"`
 
-</td>
-    </tr>
-    <tr>
-      <th>
-        <code>options.onUnhandledRequest</code> __deprecated__
-      </th>
-      <th>
-        <code>function</code>
-      </th>
-      <td>Defaults to returns:
-
-```js
-function onUnhandledRequest(request) {
-  return
-    {
-      status: 404,
-      headers: { "content-type": "application/json" },
-      body: JSON.stringify({
-        error: `Unknown route: [METHOD] [URL]`,
-      })
-    }
-  );
-}
-```
-
 </td>
     </tr>
   </tbody>

diff --git a/src/index.ts b/src/index.ts
@@ -60,12 +60,11 @@ export type {
 
 // generic handlers
 export { handleRequest } from "./middleware/handle-request";
+export { unknownRouteResponse } from "./middleware/unknown-route-response";
 
 export { createNodeMiddleware } from "./middleware/node/index";
-export {
-  createCloudflareHandler,
-  createWebWorkerHandler,
-} from "./middleware/web-worker/index";
+export { sendResponse as sendNodeResponse } from "./middleware/node/send-response";
+export { createWebWorkerHandler } from "./middleware/web-worker/index";
 export { createAWSLambdaAPIGatewayV2Handler } from "./middleware/aws-lambda/api-gateway-v2";
 
 type Constructor<T> = new (...args: any[]) => T;

diff --git a/src/middleware/README.md b/src/middleware/README.md
@@ -7,11 +7,9 @@ The `middleware` directory contains the generic HTTP handler. Each sub-directory
 ```
 middleware
 ├── handle-request.ts
-├── on-unhandled-request-default.ts
 ├── types.ts
 ├── node/
 ├── web-worker/ (Cloudflare Workers & Deno)
-└── deno/ (to be implemented)
 ```
 
 ## Generic HTTP Handler
@@ -85,5 +83,5 @@ Implementing an HTTP handler/middleware for a certain environment involves three
 2. Write a function to render an `OctokitResponse` object (e.g., as `ServerResponse` in Node.js). See [`node/send-response.ts`](node/send-response.ts) for reference.
 3. Expose an HTTP handler/middleware in the dialect of the environment which performs three steps:
    1. Parse the HTTP request using (1).
-   2. Process the `OctokitRequest` object using `handleRequest`. If the request is not handled by `handleRequest` (the request does not match any predefined route), [`onUnhandledRequestDefault`](on-unhandled-request-default.ts) can be used to generate a `404` response consistently.
+   2. Process the `OctokitRequest` object using `handleRequest`.
    3. Render the `OctokitResponse` object using (2).
diff --git a/src/middleware/aws-lambda/api-gateway-v2.ts b/src/middleware/aws-lambda/api-gateway-v2.ts
@@ -1,7 +1,6 @@
 import { parseRequest } from "./api-gateway-v2-parse-request";
 import { sendResponse } from "./api-gateway-v2-send-response";
 import { handleRequest } from "../handle-request";
-import { onUnhandledRequestDefault } from "../on-unhandled-request-default";
 import { HandlerOptions } from "../types";
 import { OAuthApp } from "../../index";
 import { ClientType, Options } from "../../types";
@@ -10,34 +9,15 @@ import type {
   APIGatewayProxyStructuredResultV2,
 } from "aws-lambda";
 
-async function onUnhandledRequestDefaultAWSAPIGatewayV2(
-  event: APIGatewayProxyEventV2
-): Promise<APIGatewayProxyStructuredResultV2> {
-  const request = parseRequest(event);
-  const response = onUnhandledRequestDefault(request);
-  return sendResponse(response);
-}
-
 export function createAWSLambdaAPIGatewayV2Handler(
   app: OAuthApp<Options<ClientType>>,
-  {
-    pathPrefix,
-    onUnhandledRequest,
-  }: HandlerOptions & {
-    onUnhandledRequest?: (
-      event: APIGatewayProxyEventV2
-    ) => Promise<APIGatewayProxyStructuredResultV2>;
-  } = {}
+  options: HandlerOptions = {}
 ) {
-  if (onUnhandledRequest) {
-    app.octokit.log.warn(
-      "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version."
-    );
-  }
-  onUnhandledRequest ??= onUnhandledRequestDefaultAWSAPIGatewayV2;
-  return async function (event: APIGatewayProxyEventV2) {
+  return async function (
+    event: APIGatewayProxyEventV2
+  ): Promise<APIGatewayProxyStructuredResultV2 | undefined> {
     const request = parseRequest(event);
-    const response = await handleRequest(app, { pathPrefix }, request);
-    return response ? sendResponse(response) : onUnhandledRequest!(event);
+    const response = await handleRequest(app, options, request);
+    return response ? sendResponse(response) : undefined;
   };
 }
diff --git a/src/middleware/handle-request.ts b/src/middleware/handle-request.ts
@@ -1,4 +1,5 @@
 import { OAuthApp } from "../index";
+import { unknownRouteResponse } from "./unknown-route-response";
 import { HandlerOptions, OctokitRequest, OctokitResponse } from "./types";
 import { ClientType, Options } from "../types";
 // @ts-ignore - requires esModuleInterop flag
@@ -8,7 +9,7 @@ export async function handleRequest(
   app: OAuthApp<Options<ClientType>>,
   { pathPrefix = "/api/github/oauth" }: HandlerOptions,
   request: OctokitRequest
-): Promise<OctokitResponse | null> {
+): Promise<OctokitResponse | undefined> {
   if (request.method === "OPTIONS") {
     return {
       status: 200,
@@ -23,23 +24,28 @@ export async function handleRequest(
 
   // request.url may include ?query parameters which we don't want for `route`
   // hence the workaround using new URL()
-  const { pathname } = new URL(request.url as string, "http://localhost");
+  let { pathname } = new URL(request.url as string, "http://localhost");
+  if (!pathname.startsWith(`${pathPrefix}/`)) {
+    return undefined;
+  }
+  pathname = pathname.slice(pathPrefix.length + 1);
+
   const route = [request.method, pathname].join(" ");
   const routes = {
-    getLogin: `GET ${pathPrefix}/login`,
-    getCallback: `GET ${pathPrefix}/callback`,
-    createToken: `POST ${pathPrefix}/token`,
-    getToken: `GET ${pathPrefix}/token`,
-    patchToken: `PATCH ${pathPrefix}/token`,
-    patchRefreshToken: `PATCH ${pathPrefix}/refresh-token`,
-    scopeToken: `POST ${pathPrefix}/token/scoped`,
-    deleteToken: `DELETE ${pathPrefix}/token`,
-    deleteGrant: `DELETE ${pathPrefix}/grant`,
+    getLogin: `GET login`,
+    getCallback: `GET callback`,
+    createToken: `POST token`,
+    getToken: `GET token`,
+    patchToken: `PATCH token`,
+    patchRefreshToken: `PATCH refresh-token`,
+    scopeToken: `POST token/scoped`,
+    deleteToken: `DELETE token`,
+    deleteGrant: `DELETE grant`,
   };
 
   // handle unknown routes
   if (!Object.values(routes).includes(route)) {
-    return null;
+    return unknownRouteResponse(request);
   }
 
   let json: any;

diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts
@@ -6,57 +6,28 @@ type ServerResponse = any;
 
 import { parseRequest } from "./parse-request";
 import { sendResponse } from "./send-response";
-import { onUnhandledRequestDefault } from "../on-unhandled-request-default";
 import { handleRequest } from "../handle-request";
 import { OAuthApp } from "../../index";
 import { HandlerOptions } from "../types";
 import { ClientType, Options } from "../../types";
 
-function onUnhandledRequestDefaultNode(
-  request: IncomingMessage,
-  response: ServerResponse
-) {
-  const octokitRequest = parseRequest(request);
-  const octokitResponse = onUnhandledRequestDefault(octokitRequest);
-  sendResponse(octokitResponse, response);
-}
-
 export function createNodeMiddleware(
   app: OAuthApp<Options<ClientType>>,
-  {
-    pathPrefix,
-    onUnhandledRequest,
-  }: HandlerOptions & {
-    onUnhandledRequest?: (
-      request: IncomingMessage,
-      response: ServerResponse
-    ) => void;
-  } = {}
+  options: HandlerOptions = {}
 ) {
-  if (onUnhandledRequest) {
-    app.octokit.log.warn(
-      "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version."
-    );
-  }
-  onUnhandledRequest ??= onUnhandledRequestDefaultNode;
   return async function (
     request: IncomingMessage,
     response: ServerResponse,
     next?: Function
   ) {
-    const octokitRequest = parseRequest(request);
-    const octokitResponse = await handleRequest(
-      app,
-      { pathPrefix },
-      octokitRequest
-    );
-
+    const octokitRequest = await parseRequest(request);
+    const octokitResponse = await handleRequest(app, options, octokitRequest);
     if (octokitResponse) {
       sendResponse(octokitResponse, response);
-    } else if (typeof next === "function") {
-      next();
+      return true;
     } else {
-      onUnhandledRequest!(request, response);
+      next?.();
+      return false;
     }
   };
 }
diff --git a/...iddleware/on-unhandled-request-default.ts → src/middleware/unknown-route-response.ts b/...iddleware/on-unhandled-request-default.ts → src/middleware/unknown-route-response.ts
@@ -1,8 +1,6 @@
-import { OctokitRequest, OctokitResponse } from "./types";
+import type { OctokitRequest } from "./types";
 
-export function onUnhandledRequestDefault(
-  request: OctokitRequest
-): OctokitResponse {
+export function unknownRouteResponse(request: OctokitRequest) {
   return {
     status: 404,
     headers: { "content-type": "application/json" },

diff --git a/src/middleware/web-worker/index.ts b/src/middleware/web-worker/index.ts
@@ -1,53 +1,17 @@
 import { parseRequest } from "./parse-request";
 import { sendResponse } from "./send-response";
 import { handleRequest } from "../handle-request";
-import { onUnhandledRequestDefault } from "../on-unhandled-request-default";
-import { OAuthApp } from "../../index";
-import { HandlerOptions } from "../types";
-import { ClientType, Options } from "../../types";
-
-async function onUnhandledRequestDefaultWebWorker(
-  request: Request
-): Promise<Response> {
-  const octokitRequest = parseRequest(request);
-  const octokitResponse = onUnhandledRequestDefault(octokitRequest);
-  return sendResponse(octokitResponse);
-}
+import type { OAuthApp } from "../../index";
+import type { HandlerOptions } from "../types";
+import type { ClientType, Options } from "../../types";
 
 export function createWebWorkerHandler<T extends Options<ClientType>>(
   app: OAuthApp<T>,
-  {
-    pathPrefix,
-    onUnhandledRequest,
-  }: HandlerOptions & {
-    onUnhandledRequest?: (request: Request) => Response | Promise<Response>;
-  } = {}
+  options: HandlerOptions = {}
 ) {
-  if (onUnhandledRequest) {
-    app.octokit.log.warn(
-      "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version."
-    );
-  }
-  onUnhandledRequest ??= onUnhandledRequestDefaultWebWorker;
-  return async function (request: Request): Promise<Response> {
-    const octokitRequest = parseRequest(request);
-    const octokitResponse = await handleRequest(
-      app,
-      { pathPrefix },
-      octokitRequest
-    );
-    return octokitResponse
-      ? sendResponse(octokitResponse)
-      : await onUnhandledRequest!(request);
+  return async function (request: Request): Promise<Response | undefined> {
+    const octokitRequest = await parseRequest(request);
+    const octokitResponse = await handleRequest(app, options, octokitRequest);
+    return octokitResponse ? sendResponse(octokitResponse) : undefined;
   };
 }
-
-/** @deprecated */
-export function createCloudflareHandler<T>(
-  ...args: Parameters<typeof createWebWorkerHandler>
-) {
-  args[0].octokit.log.warn(
-    "[@octokit/oauth-app] `createCloudflareHandler` is deprecated, use `createWebWorkerHandler` instead"
-  );
-  return createWebWorkerHandler(...args);
-}