From 890e0b0aa2af0dff2aac14461f38b97a6dfda903 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 2 Dec 2024 21:46:24 +0100 Subject: [PATCH 1/4] fix: partially reverts 644 --- packages/open-next/src/core/requestHandler.ts | 17 ++++------ .../open-next/src/core/routing/middleware.ts | 6 ++-- packages/open-next/src/core/routingHandler.ts | 33 +++++++++++-------- .../overrides/converters/aws-cloudfront.ts | 4 +-- .../src/overrides/converters/edge.ts | 7 ++-- .../src/overrides/wrappers/cloudflare-edge.ts | 6 ++-- 6 files changed, 37 insertions(+), 36 deletions(-) diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index acc7c1a32..0d107054d 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -7,9 +7,8 @@ import { runWithOpenNextRequestContext } from "utils/promise"; import { debug, error, warn } from "../adapters/logger"; import { patchAsyncStorage } from "./patchAsyncStorage"; -import { resolveProxyRequest } from "./resolve"; import { convertRes, createServerResponse } from "./routing/util"; -import type { MiddlewareOutputEvent } from "./routingHandler"; +import type { RoutingResult } from "./routingHandler"; import routingHandler, { MIDDLEWARE_HEADER_PREFIX, MIDDLEWARE_HEADER_PREFIX_LEN, @@ -34,8 +33,7 @@ export async function openNextHandler( } debug("internalEvent", internalEvent); - let routingResult: InternalResult | MiddlewareOutputEvent = { - type: "middleware", + let routingResult: InternalResult | RoutingResult = { internalEvent, isExternalRewrite: false, origin: false, @@ -51,9 +49,9 @@ export async function openNextHandler( //#endOverride const headers = - routingResult.type === "middleware" - ? routingResult.internalEvent.headers - : routingResult.headers; + "type" in routingResult + ? routingResult.headers + : routingResult.internalEvent.headers; const overwrittenResponseHeaders: Record = {}; @@ -68,7 +66,7 @@ export async function openNextHandler( } if ( - routingResult.type === "middleware" && + "isExternalRewrite" in routingResult && routingResult.isExternalRewrite === true ) { try { @@ -78,7 +76,6 @@ export async function openNextHandler( } catch (e) { error("External request failed.", e); routingResult = { - type: "middleware", internalEvent: { type: "core", rawPath: "/500", @@ -97,7 +94,7 @@ export async function openNextHandler( } } - if (routingResult.type === "core") { + if ("type" in routingResult) { // response is used only in the streaming case if (responseStreaming) { const response = createServerResponse( diff --git a/packages/open-next/src/core/routing/middleware.ts b/packages/open-next/src/core/routing/middleware.ts index 708beda4d..4562eb60d 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -23,7 +23,7 @@ const middlewareManifest = MiddlewareManifest; const middleMatch = getMiddlewareMatch(middlewareManifest); -type InternalMiddlewareEvent = InternalEvent & { +type MiddlewareEvent = InternalEvent & { responseHeaders?: Record; isExternalRewrite?: boolean; }; @@ -45,7 +45,7 @@ function defaultMiddlewareLoader() { export async function handleMiddleware( internalEvent: InternalEvent, middlewareLoader: MiddlewareLoader = defaultMiddlewareLoader, -): Promise { +): Promise { const headers = internalEvent.headers; // We bypass the middleware if the request is internal @@ -207,5 +207,5 @@ export async function handleMiddleware( cookies: internalEvent.cookies, remoteAddress: internalEvent.remoteAddress, isExternalRewrite, - } satisfies InternalMiddlewareEvent; + } satisfies MiddlewareEvent; } diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 55c2a82cd..f2d12e6a6 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -4,7 +4,12 @@ import { PrerenderManifest, RoutesManifest, } from "config/index"; -import type { InternalEvent, InternalResult, Origin } from "types/open-next"; +import type { + BaseEventOrResult, + InternalEvent, + InternalResult, + Origin, +} from "types/open-next"; import { debug } from "../adapters/logger"; import { cacheInterceptor } from "./routing/cacheInterceptor"; @@ -20,14 +25,17 @@ import { handleMiddleware } from "./routing/middleware"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; -export interface MiddlewareOutputEvent { - type: "middleware"; +export interface RoutingResult { internalEvent: InternalEvent; isExternalRewrite: boolean; origin: Origin | false; isISR: boolean; } +export interface MiddlewareResult + extends RoutingResult, + BaseEventOrResult<"middleware"> {} + // Add the locale prefix to the regex so we correctly match the rawPath const optionalLocalePrefixRegex = RoutesManifest.locales.length ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?` @@ -88,7 +96,7 @@ function applyMiddlewareHeaders( export default async function routingHandler( event: InternalEvent, -): Promise { +): Promise { // Add Next geo headers for (const [openNextGeoName, nextGeoName] of Object.entries( geoHeaderToNextHeader, @@ -112,15 +120,15 @@ export default async function routingHandler( return redirect; } - const middlewareEventOrResult = await handleMiddleware(internalEvent); - const isInternalResult = "statusCode" in middlewareEventOrResult; - if (isInternalResult) { - return middlewareEventOrResult; + const eventOrResult = await handleMiddleware(internalEvent); + const isResult = "statusCode" in eventOrResult; + if (isResult) { + return eventOrResult; } - const middlewareResponseHeaders = middlewareEventOrResult.responseHeaders; - let isExternalRewrite = middlewareEventOrResult.isExternalRewrite ?? false; - // internalEvent is `InternalEvent | InternalMiddlewareEvent` - internalEvent = middlewareEventOrResult; + const middlewareResponseHeaders = eventOrResult.responseHeaders; + let isExternalRewrite = eventOrResult.isExternalRewrite ?? false; + // internalEvent is `InternalEvent | MiddlewareEvent` + internalEvent = eventOrResult; if (!isExternalRewrite) { // First rewrite to be applied @@ -234,7 +242,6 @@ export default async function routingHandler( }); return { - type: "middleware", internalEvent, isExternalRewrite, origin: false, diff --git a/packages/open-next/src/overrides/converters/aws-cloudfront.ts b/packages/open-next/src/overrides/converters/aws-cloudfront.ts index 384ca9c53..71b8cda82 100644 --- a/packages/open-next/src/overrides/converters/aws-cloudfront.ts +++ b/packages/open-next/src/overrides/converters/aws-cloudfront.ts @@ -19,7 +19,7 @@ import { convertToQueryString, createServerResponse, } from "../../core/routing/util"; -import type { MiddlewareOutputEvent } from "../../core/routingHandler"; +import type { MiddlewareResult } from "../../core/routingHandler"; const cloudfrontBlacklistedHeaders = [ // Disallowed headers, see: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/edge-function-restrictions-all.html#function-restrictions-disallowed-headers @@ -146,7 +146,7 @@ function convertToCloudfrontHeaders( } async function convertToCloudFrontRequestResult( - result: InternalResult | MiddlewareOutputEvent, + result: InternalResult | MiddlewareResult, originalRequest: CloudFrontRequestEvent, ): Promise { if (result.type === "middleware") { diff --git a/packages/open-next/src/overrides/converters/edge.ts b/packages/open-next/src/overrides/converters/edge.ts index 530777841..366d06fe6 100644 --- a/packages/open-next/src/overrides/converters/edge.ts +++ b/packages/open-next/src/overrides/converters/edge.ts @@ -4,17 +4,14 @@ import { parseCookies } from "http/util"; import type { InternalEvent, InternalResult } from "types/open-next"; import type { Converter } from "types/overrides"; -import type { MiddlewareOutputEvent } from "../../core/routingHandler"; +import type { MiddlewareResult } from "../../core/routingHandler"; declare global { // Makes convertTo returns the request instead of fetching it. var __dangerous_ON_edge_converter_returns_request: boolean | undefined; } -const converter: Converter< - InternalEvent, - InternalResult | MiddlewareOutputEvent -> = { +const converter: Converter = { convertFrom: async (event: Request) => { const url = new URL(event.url); diff --git a/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts b/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts index b8f4ba514..a87bf558a 100644 --- a/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts +++ b/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts @@ -1,7 +1,7 @@ import type { InternalEvent, InternalResult } from "types/open-next"; import type { Wrapper, WrapperHandler } from "types/overrides"; -import type { MiddlewareOutputEvent } from "../../core/routingHandler"; +import type { MiddlewareResult } from "../../core/routingHandler"; const cfPropNameToHeaderName = { city: "x-open-next-city", @@ -17,7 +17,7 @@ interface WorkerContext { const handler: WrapperHandler< InternalEvent, - InternalResult | MiddlewareOutputEvent + InternalResult | MiddlewareResult > = async (handler, converter) => async ( @@ -65,4 +65,4 @@ export default { name: "cloudflare-edge", supportStreaming: true, edgeRuntime: true, -} satisfies Wrapper; +} satisfies Wrapper; From 7187ecada7ebecc9f43f0cd281777d20756ec16a Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 2 Dec 2024 21:58:45 +0100 Subject: [PATCH 2/4] fixup! add explicit types --- packages/open-next/src/adapters/middleware.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/open-next/src/adapters/middleware.ts b/packages/open-next/src/adapters/middleware.ts index 952d58fa7..1fac2e304 100644 --- a/packages/open-next/src/adapters/middleware.ts +++ b/packages/open-next/src/adapters/middleware.ts @@ -1,4 +1,4 @@ -import type { InternalEvent, Origin } from "types/open-next"; +import type { InternalEvent, InternalResult, Origin } from "types/open-next"; import { runWithOpenNextRequestContext } from "utils/promise"; import { debug, error } from "../adapters/logger"; @@ -10,12 +10,14 @@ import { resolveQueue, resolveTagCache, } from "../core/resolve"; -import routingHandler from "../core/routingHandler"; +import routingHandler, { type MiddlewareResult } from "../core/routingHandler"; globalThis.internalFetch = fetch; globalThis.__openNextAls = new AsyncLocalStorage(); -const defaultHandler = async (internalEvent: InternalEvent) => { +const defaultHandler = async ( + internalEvent: InternalEvent, +): Promise => { const originResolver = await resolveOriginResolver( globalThis.openNextConfig.middleware?.originResolver, ); From ad5490949039568139fbd3f95cc6c542c0745883 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 3 Dec 2024 09:10:50 +0100 Subject: [PATCH 3/4] fixup! move *Result to types --- .changeset/sharp-worms-cross.md | 5 +++++ packages/open-next/src/adapters/middleware.ts | 8 ++++++-- packages/open-next/src/core/requestHandler.ts | 7 +++++-- packages/open-next/src/core/routingHandler.ts | 14 +------------- packages/open-next/src/types/open-next.ts | 11 +++++++++++ 5 files changed, 28 insertions(+), 17 deletions(-) create mode 100644 .changeset/sharp-worms-cross.md diff --git a/.changeset/sharp-worms-cross.md b/.changeset/sharp-worms-cross.md new file mode 100644 index 000000000..adf36bb16 --- /dev/null +++ b/.changeset/sharp-worms-cross.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +fix: partially reverts 644 diff --git a/packages/open-next/src/adapters/middleware.ts b/packages/open-next/src/adapters/middleware.ts index 1fac2e304..bb9b7c2da 100644 --- a/packages/open-next/src/adapters/middleware.ts +++ b/packages/open-next/src/adapters/middleware.ts @@ -1,4 +1,8 @@ -import type { InternalEvent, InternalResult, Origin } from "types/open-next"; +import type { + InternalEvent, + InternalResult, + MiddlewareResult, +} from "types/open-next"; import { runWithOpenNextRequestContext } from "utils/promise"; import { debug, error } from "../adapters/logger"; @@ -10,7 +14,7 @@ import { resolveQueue, resolveTagCache, } from "../core/resolve"; -import routingHandler, { type MiddlewareResult } from "../core/routingHandler"; +import routingHandler from "../core/routingHandler"; globalThis.internalFetch = fetch; globalThis.__openNextAls = new AsyncLocalStorage(); diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts index 0d107054d..d938ba152 100644 --- a/packages/open-next/src/core/requestHandler.ts +++ b/packages/open-next/src/core/requestHandler.ts @@ -2,13 +2,16 @@ import { AsyncLocalStorage } from "node:async_hooks"; import type { OpenNextNodeResponse, StreamCreator } from "http/index.js"; import { IncomingMessage } from "http/index.js"; -import type { InternalEvent, InternalResult } from "types/open-next"; +import type { + InternalEvent, + InternalResult, + RoutingResult, +} from "types/open-next"; import { runWithOpenNextRequestContext } from "utils/promise"; import { debug, error, warn } from "../adapters/logger"; import { patchAsyncStorage } from "./patchAsyncStorage"; import { convertRes, createServerResponse } from "./routing/util"; -import type { RoutingResult } from "./routingHandler"; import routingHandler, { MIDDLEWARE_HEADER_PREFIX, MIDDLEWARE_HEADER_PREFIX_LEN, diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index f2d12e6a6..e7aec3f2e 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -5,10 +5,9 @@ import { RoutesManifest, } from "config/index"; import type { - BaseEventOrResult, InternalEvent, InternalResult, - Origin, + RoutingResult, } from "types/open-next"; import { debug } from "../adapters/logger"; @@ -25,17 +24,6 @@ import { handleMiddleware } from "./routing/middleware"; export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-"; export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length; -export interface RoutingResult { - internalEvent: InternalEvent; - isExternalRewrite: boolean; - origin: Origin | false; - isISR: boolean; -} - -export interface MiddlewareResult - extends RoutingResult, - BaseEventOrResult<"middleware"> {} - // Add the locale prefix to the regex so we correctly match the rawPath const optionalLocalePrefixRegex = RoutesManifest.locales.length ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?` diff --git a/packages/open-next/src/types/open-next.ts b/packages/open-next/src/types/open-next.ts index 9d045958e..af0338deb 100644 --- a/packages/open-next/src/types/open-next.ts +++ b/packages/open-next/src/types/open-next.ts @@ -95,6 +95,17 @@ export type IncludedConverter = | "sqs-revalidate" | "dummy"; +export interface RoutingResult { + internalEvent: InternalEvent; + isExternalRewrite: boolean; + origin: Origin | false; + isISR: boolean; +} + +export interface MiddlewareResult + extends RoutingResult, + BaseEventOrResult<"middleware"> {} + export type IncludedQueue = "sqs" | "sqs-lite" | "dummy"; export type IncludedIncrementalCache = "s3" | "s3-lite" | "dummy"; From 979ea333ae93bbb07c49e199069631a2d5e7fd7f Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 3 Dec 2024 09:25:45 +0100 Subject: [PATCH 4/4] fixup: fix build --- .../src/overrides/converters/aws-cloudfront.ts | 14 ++++++-------- .../open-next/src/overrides/converters/edge.ts | 8 +++++--- .../src/overrides/wrappers/cloudflare-edge.ts | 8 +++++--- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/open-next/src/overrides/converters/aws-cloudfront.ts b/packages/open-next/src/overrides/converters/aws-cloudfront.ts index 71b8cda82..bf44749d3 100644 --- a/packages/open-next/src/overrides/converters/aws-cloudfront.ts +++ b/packages/open-next/src/overrides/converters/aws-cloudfront.ts @@ -8,18 +8,16 @@ import type { CloudFrontRequestResult, } from "aws-lambda"; import { parseCookies } from "http/util"; -import type { InternalEvent, InternalResult } from "types/open-next"; +import type { + InternalEvent, + InternalResult, + MiddlewareResult, +} from "types/open-next"; import type { Converter } from "types/overrides"; import { fromReadableStream } from "utils/stream"; import { debug } from "../../adapters/logger"; -import { - convertRes, - convertToQuery, - convertToQueryString, - createServerResponse, -} from "../../core/routing/util"; -import type { MiddlewareResult } from "../../core/routingHandler"; +import { convertToQuery, convertToQueryString } from "../../core/routing/util"; const cloudfrontBlacklistedHeaders = [ // Disallowed headers, see: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/edge-function-restrictions-all.html#function-restrictions-disallowed-headers diff --git a/packages/open-next/src/overrides/converters/edge.ts b/packages/open-next/src/overrides/converters/edge.ts index 366d06fe6..6cfcf9b99 100644 --- a/packages/open-next/src/overrides/converters/edge.ts +++ b/packages/open-next/src/overrides/converters/edge.ts @@ -1,11 +1,13 @@ import { Buffer } from "node:buffer"; import { parseCookies } from "http/util"; -import type { InternalEvent, InternalResult } from "types/open-next"; +import type { + InternalEvent, + InternalResult, + MiddlewareResult, +} from "types/open-next"; import type { Converter } from "types/overrides"; -import type { MiddlewareResult } from "../../core/routingHandler"; - declare global { // Makes convertTo returns the request instead of fetching it. var __dangerous_ON_edge_converter_returns_request: boolean | undefined; diff --git a/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts b/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts index a87bf558a..02a7318eb 100644 --- a/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts +++ b/packages/open-next/src/overrides/wrappers/cloudflare-edge.ts @@ -1,8 +1,10 @@ -import type { InternalEvent, InternalResult } from "types/open-next"; +import type { + InternalEvent, + InternalResult, + MiddlewareResult, +} from "types/open-next"; import type { Wrapper, WrapperHandler } from "types/overrides"; -import type { MiddlewareResult } from "../../core/routingHandler"; - const cfPropNameToHeaderName = { city: "x-open-next-city", country: "x-open-next-country",