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

fix(cloudflare): cloudflare-streaming do not use the edge runtime #644

Merged
merged 3 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/dirty-plums-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@opennextjs/aws": patch
---

fix(cloudflare): cloudflare-streaming do not use the edge runtime
37 changes: 20 additions & 17 deletions packages/open-next/src/core/requestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,26 @@ export async function openNextHandler(
}
debug("internalEvent", internalEvent);

let preprocessResult: InternalResult | MiddlewareOutputEvent = {
internalEvent: internalEvent,
let routingResult: InternalResult | MiddlewareOutputEvent = {
type: "middleware",
internalEvent,
isExternalRewrite: false,
origin: false,
isISR: false,
};

//#override withRouting
try {
preprocessResult = await routingHandler(internalEvent);
routingResult = await routingHandler(internalEvent);
} catch (e) {
warn("Routing failed.", e);
}
//#endOverride

const headers =
"type" in preprocessResult
? preprocessResult.headers
: preprocessResult.internalEvent.headers;
routingResult.type === "middleware"
? routingResult.internalEvent.headers
: routingResult.headers;

const overwrittenResponseHeaders: Record<string, string | string[]> = {};

Expand All @@ -67,16 +68,17 @@ export async function openNextHandler(
}

if (
"isExternalRewrite" in preprocessResult &&
preprocessResult.isExternalRewrite === true
routingResult.type === "middleware" &&
routingResult.isExternalRewrite === true
) {
try {
preprocessResult = await globalThis.proxyExternalRequest.proxy(
preprocessResult.internalEvent,
routingResult = await globalThis.proxyExternalRequest.proxy(
routingResult.internalEvent,
);
} catch (e) {
error("External request failed.", e);
preprocessResult = {
routingResult = {
type: "middleware",
internalEvent: {
type: "core",
rawPath: "/500",
Expand All @@ -95,26 +97,27 @@ export async function openNextHandler(
}
}

if ("type" in preprocessResult) {
if (routingResult.type === "core") {
// response is used only in the streaming case
if (responseStreaming) {
const response = createServerResponse(
internalEvent,
headers,
responseStreaming,
);
response.statusCode = preprocessResult.statusCode;
response.statusCode = routingResult.statusCode;
response.flushHeaders();
const [bodyToConsume, bodyToReturn] = preprocessResult.body.tee();
const [bodyToConsume, bodyToReturn] = routingResult.body.tee();
for await (const chunk of bodyToConsume) {
response.write(chunk);
}
response.end();
preprocessResult.body = bodyToReturn;
routingResult.body = bodyToReturn;
}
return preprocessResult;
return routingResult;
}
const preprocessedEvent = preprocessResult.internalEvent;

const preprocessedEvent = routingResult.internalEvent;
debug("preprocessedEvent", preprocessedEvent);
const reqProps = {
method: preprocessedEvent.method,
Expand Down
19 changes: 9 additions & 10 deletions packages/open-next/src/core/routing/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ const middlewareManifest = MiddlewareManifest;

const middleMatch = getMiddlewareMatch(middlewareManifest);

type MiddlewareOutputEvent = InternalEvent & {
type InternalMiddlewareEvent = InternalEvent & {
responseHeaders?: Record<string, string | string[]>;
externalRewrite?: boolean;
isExternalRewrite?: boolean;
};

type Middleware = (request: Request) => Response | Promise<Response>;
Expand All @@ -45,7 +45,7 @@ function defaultMiddlewareLoader() {
export async function handleMiddleware(
internalEvent: InternalEvent,
middlewareLoader: MiddlewareLoader = defaultMiddlewareLoader,
): Promise<MiddlewareOutputEvent | InternalResult> {
): Promise<InternalMiddlewareEvent | InternalResult> {
const headers = internalEvent.headers;

// We bypass the middleware if the request is internal
Expand Down Expand Up @@ -142,22 +142,22 @@ export async function handleMiddleware(
statusCode: statusCode,
headers: resHeaders,
isBase64Encoded: false,
};
} satisfies InternalResult;
}

// If the middleware returned a Rewrite, set the `url` to the pathname of the rewrite
// NOTE: the header was added to `req` from above
const rewriteUrl = responseHeaders.get("x-middleware-rewrite");
let rewritten = false;
let externalRewrite = false;
let isExternalRewrite = false;
let middlewareQueryString = internalEvent.query;
let newUrl = internalEvent.url;
if (rewriteUrl) {
// If not a string, it should probably throw
if (isExternal(rewriteUrl, internalEvent.headers.host as string)) {
newUrl = rewriteUrl;
rewritten = true;
externalRewrite = true;
isExternalRewrite = true;
} else {
const rewriteUrlObject = new URL(rewriteUrl);
newUrl = rewriteUrlObject.pathname;
Expand All @@ -184,14 +184,13 @@ export async function handleMiddleware(
// transfer response body to res
const body = result.body as ReadableStream<Uint8Array>;

// await pipeReadable(result.response.body, res);
return {
type: internalEvent.type,
statusCode: statusCode,
headers: resHeaders,
body,
isBase64Encoded: false,
};
} satisfies InternalResult;
}

return {
Expand All @@ -207,6 +206,6 @@ export async function handleMiddleware(
query: middlewareQueryString,
cookies: internalEvent.cookies,
remoteAddress: internalEvent.remoteAddress,
externalRewrite,
};
isExternalRewrite,
} satisfies InternalMiddlewareEvent;
}
20 changes: 11 additions & 9 deletions packages/open-next/src/core/routingHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ 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";
internalEvent: InternalEvent;
isExternalRewrite: boolean;
origin: Origin | false;
Expand Down Expand Up @@ -110,17 +112,16 @@ export default async function routingHandler(
return redirect;
}

const middleware = await handleMiddleware(internalEvent);
let middlewareResponseHeaders: Record<string, string | string[]> = {};
if ("statusCode" in middleware) {
return middleware;
const middlewareEventOrResult = await handleMiddleware(internalEvent);
const isInternalResult = "statusCode" in middlewareEventOrResult;
if (isInternalResult) {
return middlewareEventOrResult;
}
middlewareResponseHeaders = middleware.responseHeaders || {};
internalEvent = middleware;

// At this point internalEvent is an InternalEvent or a MiddlewareOutputEvent
const middlewareResponseHeaders = middlewareEventOrResult.responseHeaders;
let isExternalRewrite = middlewareEventOrResult.isExternalRewrite ?? false;
// internalEvent is `InternalEvent | InternalMiddlewareEvent`
internalEvent = middlewareEventOrResult;

let isExternalRewrite = middleware.externalRewrite ?? false;
if (!isExternalRewrite) {
// First rewrite to be applied
const beforeRewrites = handleRewrites(
Expand Down Expand Up @@ -233,6 +234,7 @@ export default async function routingHandler(
});

return {
type: "middleware",
internalEvent,
isExternalRewrite,
origin: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ async function convertFromCloudFrontRequestEvent(
};
}

type MiddlewareEvent = {
type: "middleware";
} & MiddlewareOutputEvent;

function convertToCloudfrontHeaders(
headers: Record<string, OutgoingHttpHeader>,
directResponse?: boolean,
Expand Down Expand Up @@ -150,7 +146,7 @@ function convertToCloudfrontHeaders(
}

async function convertToCloudFrontRequestResult(
result: InternalResult | MiddlewareEvent,
result: InternalResult | MiddlewareOutputEvent,
originalRequest: CloudFrontRequestEvent,
): Promise<CloudFrontRequestResult> {
if (result.type === "middleware") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import zlib from "node:zlib";

import type { APIGatewayProxyEventV2 } from "aws-lambda";
import type { StreamCreator } from "http/index";
import type { WrapperHandler } from "types/overrides";
import type { Wrapper, WrapperHandler } from "types/overrides";

import { debug, error } from "../../adapters/logger";
import type {
Expand Down Expand Up @@ -119,4 +119,4 @@ export default {
wrapper: handler,
name: "aws-lambda-streaming",
supportStreaming: true,
};
} satisfies Wrapper;
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import type { InternalEvent, InternalResult } from "types/open-next";
import type { WrapperHandler } from "types/overrides";
import type { Wrapper, WrapperHandler } from "types/overrides";

import { Writable } from "node:stream";
import type { StreamCreator } from "http/index";
import type { MiddlewareOutputEvent } from "../../core/routingHandler";

const handler: WrapperHandler<
InternalEvent,
InternalResult | ({ type: "middleware" } & MiddlewareOutputEvent)
> =
const handler: WrapperHandler<InternalEvent, InternalResult> =
async (handler, converter) =>
async (
request: Request,
Expand Down Expand Up @@ -73,5 +69,4 @@ export default {
wrapper: handler,
name: "cloudflare-streaming",
supportStreaming: true,
edgeRuntime: true,
};
} satisfies Wrapper;
6 changes: 3 additions & 3 deletions packages/open-next/src/overrides/wrappers/cloudflare.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { InternalEvent, InternalResult } from "types/open-next";
import type { WrapperHandler } from "types/overrides";
import type { Wrapper, WrapperHandler } from "types/overrides";

import type { MiddlewareOutputEvent } from "../../core/routingHandler";

Expand All @@ -17,7 +17,7 @@ interface WorkerContext {

const handler: WrapperHandler<
InternalEvent,
InternalResult | ({ type: "middleware" } & MiddlewareOutputEvent)
InternalResult | MiddlewareOutputEvent
> =
async (handler, converter) =>
async (
Expand Down Expand Up @@ -65,4 +65,4 @@ export default {
name: "cloudflare",
supportStreaming: true,
edgeRuntime: true,
};
} satisfies Wrapper<InternalEvent, InternalResult | MiddlewareOutputEvent>;
6 changes: 3 additions & 3 deletions packages/open-next/src/overrides/wrappers/dummy.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { WrapperHandler } from "types/overrides";
import type { Wrapper, WrapperHandler } from "types/overrides";

const dummyWrapper: WrapperHandler = async () => async () => undefined;

export default {
name: "dummy",
handler: dummyWrapper,
wrapper: dummyWrapper,
supportStreaming: false,
};
} satisfies Wrapper;
4 changes: 2 additions & 2 deletions packages/open-next/src/overrides/wrappers/node.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createServer } from "node:http";

import type { StreamCreator } from "http/index";
import type { WrapperHandler } from "types/overrides";
import type { Wrapper, WrapperHandler } from "types/overrides";

import { debug, error } from "../../adapters/logger";

Expand Down Expand Up @@ -66,4 +66,4 @@ export default {
wrapper,
name: "node",
supportStreaming: true,
};
} satisfies Wrapper;
8 changes: 4 additions & 4 deletions packages/tests-unit/tests/core/routing/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe("handleMiddleware", () => {
responseHeaders: {
"x-middleware-rewrite": "http://localhost/rewrite",
},
externalRewrite: false,
isExternalRewrite: false,
});
});

Expand Down Expand Up @@ -176,7 +176,7 @@ describe("handleMiddleware", () => {
__nextDataReq: "1",
newKey: "value",
},
externalRewrite: false,
isExternalRewrite: false,
});
});

Expand All @@ -201,7 +201,7 @@ describe("handleMiddleware", () => {
responseHeaders: {
"x-middleware-rewrite": "http://external/rewrite",
},
externalRewrite: true,
isExternalRewrite: true,
});
});

Expand All @@ -221,7 +221,7 @@ describe("handleMiddleware", () => {
"custom-header": "value",
},
responseHeaders: {},
externalRewrite: false,
isExternalRewrite: false,
});
});

Expand Down