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: edge rendering failing due to undefined req.originalRequest #58450

Open
wants to merge 6 commits into
base: canary
Choose a base branch
from

Conversation

james-elicx
Copy link

Fixing a bug

What?

#57815 introduced a bug where edge rendering fails due to the assumption that req is only of top NodeNextRequest and cannot be WebNextRequest, and therefore assumes that req.originalRequest is defined. In other parts of this file, it correctly falls back to using req when there is no originalRequest property defined on the object. Not falling back results in trying to access a property on undefined; req.originalRequest.socket, which throws an error and causes rendering to fail when deploying to Cloudflare Pages.

const { originalRequest } = req as NodeNextRequest
req.headers['x-forwarded-proto'] ??= (originalRequest.socket as TLSSocket)

Here is an example of how this is treated elsewhere in this file, and how it should be treated where this bug occurs. Note how it falls back to using req as WebNextRequest.

result = await routeModule.render(
(req as NodeNextRequest).originalRequest ?? (req as WebNextRequest),
(res as NodeNextResponse).originalResponse ??
(res as WebNextResponse),
{ page: pathname, params: opts.params, query, renderOpts }
)

We have had to manually add this patch to @cloudflare/next-on-pages for deployments on Cloudflare Pages to continue working (cloudflare/next-on-pages#534).

I tried to add tests but was unable to actually run any of the test suites on my MacBook or Desktop (WSL), so was unable to do so. This fix is pretty straightforward though and is just doing what is done elsewhere in this file, so hopefully that won't be a blocker.

Why?

How?

Closes NEXT-
Fixes #58265

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants