Skip to content
Draft
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
146 changes: 3 additions & 143 deletions packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {
import { getModifiedCookieValues } from '../web/spec-extension/adapters/request-cookies'

import {
JSON_CONTENT_TYPE_HEADER,
NEXT_CACHE_REVALIDATED_TAGS_HEADER,
NEXT_CACHE_REVALIDATE_TAG_TOKEN_HEADER,
} from '../../lib/constants'
Expand All @@ -52,7 +51,6 @@ import { RequestCookies, ResponseCookies } from '../web/spec-extension/cookies'
import { HeadersAdapter } from '../web/spec-extension/adapters/headers'
import { fromNodeOutgoingHttpHeaders } from '../web/utils'
import {
selectWorkerForForwarding,
type ServerModuleMap,
getServerActionsManifest,
getServerModuleMap,
Expand Down Expand Up @@ -204,119 +202,6 @@ function addRevalidationHeader(
}
}

/**
* Forwards a server action request to a separate worker. Used when the requested action is not available in the current worker.
*/
async function createForwardedActionResponse(
req: BaseNextRequest,
res: BaseNextResponse,
host: Host,
workerPathname: string,
basePath: string
) {
if (!host) {
throw new Error(
'Invariant: Missing `host` header from a forwarded Server Actions request.'
)
}

const forwardedHeaders = getForwardedHeaders(req, res)

// indicate that this action request was forwarded from another worker
// we use this to skip rendering the flight tree so that we don't update the UI
// with the response from the forwarded worker
forwardedHeaders.set('x-action-forwarded', '1')

// TODO: Remove __NEXT_PRIVATE_ORIGIN
let origin: string | undefined = process.env.__NEXT_PRIVATE_ORIGIN
if (origin === undefined) {
const initUrl = getRequestMeta(req, 'initURL')
if (initUrl !== undefined) {
try {
const parsedUrl = new URL(initUrl)
origin = parsedUrl.origin
} catch (error) {
throw new Error(
'Could not determine origin for forwarded Server Actions request. This can happen if port or hostname are not configured for this server.',
{ cause: error }
)
}
} else {
throw new InvariantError('Missing initURL')
}
}

const fetchUrl = new URL(`${origin}${basePath}${workerPathname}`)

try {
let body: BodyInit | ReadableStream<Uint8Array> | undefined
if (
// The type check here ensures that `req` is correctly typed, and the
// environment variable check provides dead code elimination.
process.env.NEXT_RUNTIME === 'edge' &&
isWebNextRequest(req)
) {
if (!req.body) {
throw new Error('Invariant: missing request body.')
}

body = req.body
} else if (
// The type check here ensures that `req` is correctly typed, and the
// environment variable check provides dead code elimination.
process.env.NEXT_RUNTIME !== 'edge' &&
isNodeNextRequest(req)
) {
body = req.stream()
} else {
throw new Error('Invariant: Unknown request type.')
}

// Forward the request to the new worker
const response = await fetch(fetchUrl, {
method: 'POST',
body,
duplex: 'half',
headers: forwardedHeaders,
redirect: 'manual',
next: {
// @ts-ignore
internal: 1,
},
})

if (
response.headers.get('content-type')?.startsWith(RSC_CONTENT_TYPE_HEADER)
) {
// copy the headers from the redirect response to the response we're sending
for (const [key, value] of response.headers) {
if (!actionsForbiddenHeaders.includes(key)) {
res.setHeader(key, value)
}
}

return new FlightRenderResult(response.body!)
}

// Since we aren't consuming the response body, we cancel it to avoid memory leaks
response.body?.cancel()

// Pass the action-not-found marker through so the client throws
// UnrecognizedActionError instead of a generic "unexpected response".
if (response.headers.get(NEXT_ACTION_NOT_FOUND_HEADER) === '1') {
res.setHeader(NEXT_ACTION_NOT_FOUND_HEADER, '1')
res.setHeader('content-type', 'text/plain')
res.statusCode = 404
return RenderResult.fromStatic('Server action not found.', 'text/plain')
}
} catch (err) {
// we couldn't stream the forwarded response, so we'll just return an empty response
console.error(`failed to forward action response`, err)
}

return RenderResult.fromStatic('{}', JSON_CONTENT_TYPE_HEADER)
}

/**
* Returns the parsed redirect URL if we deem that it is hosted by us.
*
Expand Down Expand Up @@ -600,7 +485,6 @@ export async function handleAction({
metadata: AppPageRenderResultMetadata
}): Promise<HandleActionResult> {
const contentType = req.headers['content-type']
const { page } = ctx.renderOpts
const serverModuleMap = getServerModuleMap()

const {
Expand Down Expand Up @@ -754,7 +638,6 @@ export async function handleAction({
'no-cache, no-store, max-age=0, must-revalidate'
)

const actionWasForwarded = Boolean(req.headers['x-action-forwarded'])
// A fetch action without a router state tree cannot produce a Flight patch
// for the currently rendered page. This occurs when the client dispatches an
// action directly to a different route, so only execute the action without
Expand All @@ -768,30 +651,7 @@ export async function handleAction({
requestStore.fallbackParams != null &&
typeof ctx.renderOpts.postponed === 'string'
const shouldSkipPageRendering =
actionWasForwarded || isActionOnlyRequest || isActionOnlyFallbackRequest

// Only attempt to forward if this request has not already been forwarded.
// Otherwise middleware that rewrites the action POST can cause the receiving
// worker to forward again, looping indefinitely.
if (actionId && !actionWasForwarded) {
const forwardedWorker = selectWorkerForForwarding(actionId, page)

// If forwardedWorker is truthy, it means there isn't a worker for the
// action in the current handler, so we forward the request to a worker that
// has the action.
if (forwardedWorker) {
return {
type: 'done',
result: await createForwardedActionResponse(
req,
res,
host,
forwardedWorker,
ctx.renderOpts.basePath
),
}
}
}
isActionOnlyRequest || isActionOnlyFallbackRequest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing Server Action forwarding breaks invoking a server action obtained indirectly at runtime (returned from another action) from a route that does not register that action, causing "Failed to find Server Action"/404 instead of executing it.

Fix on Vercel


try {
return await actionAsyncStorage.run(
Expand Down Expand Up @@ -906,7 +766,7 @@ export async function handleAction({
[],
workStore,
requestStore,
actionWasForwarded
false
)

const formState = await decodeFormState(
Expand Down Expand Up @@ -1115,7 +975,7 @@ export async function handleAction({
[],
workStore,
requestStore,
actionWasForwarded
false
)

const formState = await decodeFormState(
Expand Down
37 changes: 0 additions & 37 deletions packages/next/src/server/app-render/manifests-singleton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { DeepReadonly } from '../../shared/lib/deep-readonly'
import { InvariantError } from '../../shared/lib/invariant-error'
import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths'
import { pathHasPrefix } from '../../shared/lib/router/utils/path-has-prefix'
import { removePathPrefix } from '../../shared/lib/router/utils/remove-path-prefix'
import { mightBeServerReferenceId } from '../../shared/lib/server-reference-info'
import { wellKnownProperties } from '../../shared/lib/utils/reflect-utils'
import { createServerActionRoutingKey } from '../../shared/lib/server-action-routing-key'
Expand Down Expand Up @@ -301,42 +300,6 @@ function normalizeWorkerPageName(pageName: string) {
return 'app' + pageName
}

/**
* Converts a bundlePath (relative path to the entrypoint) to a routable page
* name.
*/
function denormalizeWorkerPageName(bundlePath: string) {
return normalizeAppPath(removePathPrefix(bundlePath, 'app'))
}

/**
* Checks if the requested action has a worker for the current page.
* If not, it returns the first worker that has a handler for the action.
*/
export function selectWorkerForForwarding(
actionId: string,
pageName: string
): string | undefined {
const serverActionsManifest = getServerActionsManifest()
const workers =
serverActionsManifest[
process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node'
][actionId]?.workers

// There are no workers to handle this action, nothing to forward to.
if (!workers) {
return
}

// If there is an entry for the current page, we don't need to forward.
if (workers[normalizeWorkerPageName(pageName)]) {
return
}

// Otherwise, grab the first worker that has a handler for this action id.
return denormalizeWorkerPageName(Object.keys(workers)[0])
}

export function getServerActionRoutingKeysForPage(
pageName: string
): readonly string[] | undefined {
Expand Down
41 changes: 37 additions & 4 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,12 @@ describe('app-dir action handling', () => {
const cliOutputIndex = next.cliOutput.length
const browser = await next.browser(`/delayed-action/${runtime}`)
const actionRequestPaths: string[] = []
let actionRequestHeaders: Record<string, string> | undefined
let actionRequest:
| {
headers: Record<string, string>
body: string | null
}
| undefined

browser.on('request', (request) => {
const headers = request.headers()
Expand All @@ -926,7 +931,10 @@ describe('app-dir action handling', () => {
headers['next-action'] !== undefined
) {
actionRequestPaths.push(new URL(request.url()).pathname)
actionRequestHeaders = headers
actionRequest = {
headers,
body: request.postData(),
}
}
})

Expand Down Expand Up @@ -958,13 +966,38 @@ describe('app-dir action handling', () => {
expect(await browser.hasElementByCssSelector('#other-page')).toBe(true)

expect(actionRequestPaths).toEqual([`/delayed-action/${runtime}`])
expect(actionRequestHeaders?.['next-action-only']).toBeUndefined()
expect(actionRequestHeaders?.['next-router-state-tree']).toBeUndefined()
expect(actionRequest?.headers['next-action-only']).toBeUndefined()
expect(actionRequest?.headers['next-router-state-tree']).toBeUndefined()

// make sure we didn't get any errors in the console
expect(next.cliOutput.slice(cliOutputIndex)).not.toContain(
'Failed to find Server Action'
)

if (actionRequest === undefined) {
throw new Error('Failed to capture Server Action request')
}

// A valid action sent to the wrong route must fail instead of being
// forwarded to a route that bundles it.
const wronglyRoutedResponse = await next.fetch(
`/delayed-action/${runtime}/other`,
{
method: 'POST',
headers: {
accept: actionRequest.headers.accept,
'content-type': actionRequest.headers['content-type'],
'next-action': actionRequest.headers['next-action'],
origin: next.url,
},
body: actionRequest.body,
}
)

expect(wronglyRoutedResponse.status).toBe(404)
expect(
wronglyRoutedResponse.headers.get('x-nextjs-action-not-found')
).toBe('1')
}
)

Expand Down
Loading