From cc32524f31e7ac4b5bf1bf62d4b6b7adebbbcf64 Mon Sep 17 00:00:00 2001 From: Baoshan Sheng Date: Wed, 7 Sep 2022 10:45:13 +0800 Subject: [PATCH 1/2] refactor(middleware): deprecate onUnhandledRequest middleware option feat(middleware): export `handleRequest`, `OctokitRequest`, and `OctokitResponse` for 3rd-party middlewares --- README.md | 6 +-- src/index.ts | 11 ++++++ src/middleware/aws-lambda/api-gateway-v2.ts | 12 ++++-- src/middleware/node/index.ts | 10 ++++- src/middleware/web-worker/index.ts | 10 ++++- test/deprecations.test.ts | 41 ++++++++++++++++++++- test/smoke.test.ts | 6 ++- 7 files changed, 84 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c3898ed7d..ce887fc1b 100644 --- a/README.md +++ b/README.md @@ -939,7 +939,7 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - options.onUnhandledRequest + options.onUnhandledRequest __deprecated__ function @@ -1029,7 +1029,7 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - options.onUnhandledRequest + options.onUnhandledRequest __deprecated__ function @@ -1119,7 +1119,7 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - options.onUnhandledRequest + options.onUnhandledRequest __deprecated__ function diff --git a/src/index.ts b/src/index.ts index a129bb2ba..353d4de5b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -50,6 +50,17 @@ import type { Options, State, } from "./types"; + +// types required by external handlers (aws-lambda, etc) +export type { + HandlerOptions, + OctokitRequest, + OctokitResponse, +} from "./middleware/types"; + +// generic handlers +export { handleRequest } from "./middleware/handle-request"; + export { createNodeMiddleware } from "./middleware/node/index"; export { createCloudflareHandler, diff --git a/src/middleware/aws-lambda/api-gateway-v2.ts b/src/middleware/aws-lambda/api-gateway-v2.ts index 873c9ae05..7eea54ac2 100644 --- a/src/middleware/aws-lambda/api-gateway-v2.ts +++ b/src/middleware/aws-lambda/api-gateway-v2.ts @@ -4,7 +4,7 @@ import { handleRequest } from "../handle-request"; import { onUnhandledRequestDefault } from "../on-unhandled-request-default"; import { HandlerOptions } from "../types"; import { OAuthApp } from "../../index"; -import { Options, ClientType } from "../../types"; +import { ClientType, Options } from "../../types"; import type { APIGatewayProxyEventV2, APIGatewayProxyStructuredResultV2, @@ -22,16 +22,22 @@ export function createAWSLambdaAPIGatewayV2Handler( app: OAuthApp>, { pathPrefix, - onUnhandledRequest = onUnhandledRequestDefaultAWSAPIGatewayV2, + onUnhandledRequest, }: HandlerOptions & { onUnhandledRequest?: ( event: APIGatewayProxyEventV2 ) => Promise; } = {} ) { + 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) { const request = parseRequest(event); const response = await handleRequest(app, { pathPrefix }, request); - return response ? sendResponse(response) : onUnhandledRequest(event); + return response ? sendResponse(response) : onUnhandledRequest!(event); }; } diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts index 9bc05f71f..d152ede38 100644 --- a/src/middleware/node/index.ts +++ b/src/middleware/node/index.ts @@ -25,7 +25,7 @@ export function createNodeMiddleware( app: OAuthApp>, { pathPrefix, - onUnhandledRequest = onUnhandledRequestDefaultNode, + onUnhandledRequest, }: HandlerOptions & { onUnhandledRequest?: ( request: IncomingMessage, @@ -33,6 +33,12 @@ export function createNodeMiddleware( ) => void; } = {} ) { + 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, @@ -50,7 +56,7 @@ export function createNodeMiddleware( } else if (typeof next === "function") { next(); } else { - onUnhandledRequest(request, response); + onUnhandledRequest!(request, response); } }; } diff --git a/src/middleware/web-worker/index.ts b/src/middleware/web-worker/index.ts index 68511336a..8e91975b3 100644 --- a/src/middleware/web-worker/index.ts +++ b/src/middleware/web-worker/index.ts @@ -18,11 +18,17 @@ export function createWebWorkerHandler>( app: OAuthApp, { pathPrefix, - onUnhandledRequest = onUnhandledRequestDefaultWebWorker, + onUnhandledRequest, }: HandlerOptions & { onUnhandledRequest?: (request: Request) => Response | Promise; } = {} ) { + 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 { const octokitRequest = parseRequest(request); const octokitResponse = await handleRequest( @@ -32,7 +38,7 @@ export function createWebWorkerHandler>( ); return octokitResponse ? sendResponse(octokitResponse) - : await onUnhandledRequest(request); + : await onUnhandledRequest!(request); }; } diff --git a/test/deprecations.test.ts b/test/deprecations.test.ts index 7d9c9e871..6b521895d 100644 --- a/test/deprecations.test.ts +++ b/test/deprecations.test.ts @@ -1,7 +1,13 @@ import { URL } from "url"; import * as nodeFetch from "node-fetch"; import fromEntries from "fromentries"; -import { createCloudflareHandler, OAuthApp } from "../src"; +import { + createAWSLambdaAPIGatewayV2Handler, + createCloudflareHandler, + createNodeMiddleware, + createWebWorkerHandler, + OAuthApp, +} from "../src"; import { Octokit } from "@octokit/core"; describe("deprecations", () => { @@ -52,4 +58,37 @@ describe("deprecations", () => { expect(url.searchParams.get("state")).toMatch(/^\w+$/); expect(url.searchParams.get("scope")).toEqual(null); }); + + it("`onUnhandledRequest` is deprecated and will be removed from the next major version", async () => { + const warn = jest.fn().mockResolvedValue(undefined); + const handleRequest = createAWSLambdaAPIGatewayV2Handler( + new OAuthApp({ + clientType: "github-app", + clientId: "client_id_123", + clientSecret: "client_secret_456", + Octokit: Octokit.defaults({ + log: { + debug: () => undefined, + info: () => undefined, + warn, + error: () => undefined, + }, + }), + }), + { + onUnhandledRequest: async (request) => { + return { + statusCode: 404, + headers: {}, + body: "", + }; + }, + } + ); + + expect(warn.mock.calls.length).toEqual(1); + expect(warn.mock.calls[0][0]).toEqual( + "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." + ); + }); }); diff --git a/test/smoke.test.ts b/test/smoke.test.ts index 2cb6c4ec9..4d2264c5a 100644 --- a/test/smoke.test.ts +++ b/test/smoke.test.ts @@ -1,4 +1,4 @@ -import { OAuthApp } from "../src"; +import { handleRequest, OAuthApp } from "../src"; describe("Smoke test", () => { it("OAuthApp is a function", () => { @@ -12,4 +12,8 @@ describe("Smoke test", () => { it("OAuthApp.VERSION is set", () => { expect(OAuthApp.VERSION).toEqual("0.0.0-development"); }); + + it("handleRequest is a function", () => { + expect(typeof handleRequest).toEqual("function"); + }); }); From 720032b083b92cbfb26205539dc775ce3566502a Mon Sep 17 00:00:00 2001 From: Baoshan Sheng Date: Thu, 29 Sep 2022 18:52:38 +0800 Subject: [PATCH 2/2] docs: export handleRequest function --- README.md | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/README.md b/README.md index ce887fc1b..5c1db4475 100644 --- a/README.md +++ b/README.md @@ -1145,6 +1145,81 @@ function onUnhandledRequest(request) { +### Build Custom Middlewares + +When above middlewares do not meet your needs, you can build your own +using the exported `handleRequest` function. + +[`handleRequest`](./src/middleware/handle-request.ts) function is an abstract HTTP handler which accepts an `OctokitRequest` and returns an `OctokitResponse` if the request matches any predefined route. + +> Different environments (e.g., Node.js, Cloudflare Workers, Deno, etc.) exposes different APIs when processing HTTP requests (e.g., [`IncomingMessage`](https://nodejs.org/api/http.html#http_class_http_incomingmessage) for Node.js, [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request) for Cloudflare workers, etc.). Two HTTP-related types ([`OctokitRequest` and `OctokitResponse`](.src/middleware/types.ts)) are generalized to make an abstract HTTP handler possible. + +To share the behavior and capability with the existing Node.js middleware (and be compatible with [OAuth user authentication strategy in the browser](https://github.com/octokit/auth-oauth-user-client.js)), it is better to implement your HTTP handler/middleware based on `handleRequest` function. + +`handleRequest` function takes three parameters: + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ name + + type + + description +
+ app + + OAuthApp instance + + Required. +
+ options.pathPrefix + + string + + +All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"` + +
+ request + + OctokitRequest + + Generalized HTTP request in `OctokitRequest` type. +
+ +Implementing an HTTP handler/middleware for a certain environment involves three steps: + +1. Write a function to parse the HTTP request (e.g., `IncomingMessage` in Node.js) into an `OctokitRequest` object. See [`node/parse-request.ts`](.src/middleware/node/parse-request.ts) for reference. +2. Write a function to render an `OctokitResponse` object (e.g., as `ServerResponse` in Node.js). See [`node/send-response.ts`](.src/middleware/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`. + 3. Render the `OctokitResponse` object using (2). + ## Contributing See [CONTRIBUTING.md](CONTRIBUTING.md)