Unexpected middleware behavior: Cannot use Astro.rewrite after the request body has been read

[xriter]

Astro Info

Astro                    v5.7.13
Node                     v20.19.1
System                   Linux (x64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             none

Describe the Bug

In my middleware there might be some intentional rewrites. But I ran into some unexpected behavior where I got the error:

Astro.rewrite() cannot be used if the request body has already been read. If you need to read the body, first clone the request.

I created a minimal reproducible example on Stackblitz.

• Check out the src/middleware.ts file.
• Then open the preview and submit the form.
• The error will show.

I don't understand why though. The error mentions "If you need to read the body, first clone the request", but I am doing that. Why/where is Astro.rewrite() called anyway?

Am I misunderstanding this, or could this be a bug?
Perhaps someone could enlighten me? 😇

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-8mju777p?file=src%2Fmiddleware.ts

[ematipico]

This is a bug about how we handle the carried paylod inside sequence. It should be easy to fix, however we should make sure that the tests still pass

astro/packages/astro/src/core/middleware/sequence.ts

Line 85 in fbcfa68

return next(payload ?? carriedPayload);

I tried using only carriedPayload and it seems to fix the issue, however we should make sure that composing multiple middleware functions with different payloads still yields the expected result.

[xriter]

@ematipico I don't exactly see what the issue is with payload ?? carriedPayload. Could you explain what's wrong with payload when it reaches this point? Is it because this particular payload has the bodyUsed as true?

[Lofty-Brambles]

Is this a bug? next accepts a rewritePayload that rewrites the request without triggering a render. That is what's causing this error, if I'm not wrong?

[xriter]

I am trying to grasp how the whole middleware/rewrite flow is implemented in Astro, but it is scattered around so many files that I am having a hard time.

For what it is worth:
I found based on the sequence implementation, that there are sort of two next functions. The one that the individual sequence handlers call (basically to call the next handler in the sequence), and then there is the 'final' next. And that one is eventually doing the actual rewriting. I can't come up with a conclusion based off this though.

[xriter]

In case this might help for debugging this issue:

I tried creating a simplified version of the sequence function.

export function simplifiedSequence(
    ...handlers: MiddlewareHandler[]
): MiddlewareHandler {
    return defineMiddleware((context, next) => {
        /** Used to carry the rerouting payload across middleware functions. */
        let finalRewriteTo: string;

        function runHandler(i: number) {
            const handler: any = handlers[i];

            return handler(context, async (rewriteTo?: string) => {
                if (i < handlers.length - 1) {
                    if (rewriteTo) {
                        finalRewriteTo = rewriteTo;
                    }
                    return runHandler(i + 1);
                } else {
                    return next(finalRewriteTo);
                }
            });
        }

        return runHandler(0);
    });
}

In the original function, there are these particular lines:

astro/packages/astro/src/core/middleware/sequence.ts

Lines 39 to 49 in fbcfa68

	let newRequest;
	if (payload instanceof Request) {
	newRequest = payload;
	} else if (payload instanceof URL) {
	newRequest = new Request(payload, handleContext.request);
	} else {
	newRequest = new Request(
	new URL(payload, handleContext.url.origin),
	handleContext.request,
	);
	}

astro/packages/astro/src/core/middleware/sequence.ts

Lines 77 to 80 in fbcfa68

	handleContext.request = newRequest;
	handleContext.url = new URL(newRequest.url);
	handleContext.cookies = new AstroCookies(newRequest);
	handleContext.params = getParams(routeData, pathname);

As I understand it, the purpose of creating newRequest here is to ensure that any middleware handlers after a handler that did a rewrite, see an updated context (where the request and URL reflect the rewritten location).

However, it seems that all middleware handlers run first, and only then the actual rewrite happens (triggered by the final next() call (I think?)). So I am not sure if updating the context mid-sequence is useful in the first place, since the actual rewrite hasn’t technically occurred yet?

So this raises the question: Should the context be updated for the remaining handlers in the sequence, even though the rewrite hasn’t been executed yet?

This simplified version of the sequence function doesn't solve the issue though. So I think the request body is still being read somewhere after the sequence function.

My current guess is that the issue is somewhere in the lastNext function. Something with copying the request here might consume the body.

astro/packages/astro/src/core/render-context.ts

Lines 154 to 265 in fbcfa68

	const lastNext = async (ctx: APIContext, payload?: RewritePayload) => {
	if (payload) {
	const oldPathname = this.pathname;
	pipeline.logger.debug('router', 'Called rewriting to:', payload);
	// we intentionally let the error bubble up
	const {
	routeData,
	componentInstance: newComponent,
	pathname,
	newUrl,
	} = await pipeline.tryRewrite(payload, this.request);
	// This is a case where the user tries to rewrite from a SSR route to a prerendered route (SSG).
	// This case isn't valid because when building for SSR, the prerendered route disappears from the server output because it becomes an HTML file,
	// so Astro can't retrieve it from the emitted manifest.
	if (
	this.pipeline.serverLike === true &&
	this.routeData.prerender === false &&
	routeData.prerender === true
	) {
	throw new AstroError({
	...ForbiddenRewrite,
	message: ForbiddenRewrite.message(this.pathname, pathname, routeData.component),
	hint: ForbiddenRewrite.hint(routeData.component),
	});
	}
	this.routeData = routeData;
	componentInstance = newComponent;
	if (payload instanceof Request) {
	this.request = payload;
	} else {
	this.request = copyRequest(
	newUrl,
	this.request,
	// need to send the flag of the previous routeData
	routeData.prerender,
	this.pipeline.logger,
	this.routeData.route,
	);
	}
	this.isRewriting = true;
	this.url = new URL(this.request.url);
	this.params = getParams(routeData, pathname);
	this.pathname = pathname;
	this.status = 200;
	setOriginPathname(this.request, oldPathname);
	}
	let response: Response;
	if (!ctx.isPrerendered) {
	const { action, setActionResult, serializeActionResult } = getActionContext(ctx);
	if (action?.calledFrom === 'form') {
	const actionResult = await action.handler();
	setActionResult(action.name, serializeActionResult(actionResult));
	}
	}
	switch (this.routeData.type) {
	case 'endpoint': {
	response = await renderEndpoint(
	componentInstance as any,
	ctx,
	this.routeData.prerender,
	logger,
	);
	break;
	}
	case 'redirect':
	return renderRedirect(this);
	case 'page': {
	const result = await this.createResult(componentInstance!, actionApiContext);
	try {
	response = await renderPage(
	result,
	componentInstance?.default as any,
	props,
	slots,
	streaming,
	this.routeData,
	);
	} catch (e) {
	// If there is an error in the page's frontmatter or instantiation of the RenderTemplate fails midway,
	// we signal to the rest of the internals that we can ignore the results of existing renders and avoid kicking off more of them.
	result.cancelled = true;
	throw e;
	}
	// Signal to the i18n middleware to maybe act on this response
	response.headers.set(ROUTE_TYPE_HEADER, 'page');
	// Signal to the error-page-rerouting infra to let this response pass through to avoid loops
	if (this.routeData.route === '/404' || this.routeData.route === '/500') {
	response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
	}
	if (this.isRewriting) {
	response.headers.set(REWRITE_DIRECTIVE_HEADER_KEY, REWRITE_DIRECTIVE_HEADER_VALUE);
	}
	break;
	}
	case 'fallback': {
	return new Response(null, { status: 500, headers: { [ROUTE_TYPE_HEADER]: 'fallback' } });
	}
	}
	// We need to merge the cookies from the response back into this.cookies
	// because they may need to be passed along from a rewrite.
	const responseCookies = getCookiesFromResponse(response);
	if (responseCookies) {
	cookies.merge(responseCookies);
	}
	return response;
	};

Any help is welcome 🕵

[Lofty-Brambles]

In your example, what's the idea behind src/middleware.ts#L7 returning /form as a param to next?

[xriter]

@Lofty-Brambles

In your example, what's the idea behind src/middleware.ts#L7 returning /form as a param to next?

The actual implementation in the website is more advanced and has a logical use-case. But I ran into this issue. So I created this heavily simplified sample just to illustrate how one can run into the problem.

In short you could say: the rewrite in this middleware handler allows the /form route to be also accessed from /.
So writing return next('/form') will cause the rewrite.

Another example could be:
I have pages/mypage.astro, and I want this page to be accessible via the routes:

• /mypage
• /my-page
• /my-awesome-page
• /my-outdated-url
• /my-localized-url

[xriter]

Update: So I tried creating the most minimal reproduction locally and I found that the issue might actually be with Bun (which I use to run astro). Somehow Bun is already consuming the request body somewhere?

// src/middleware.ts
import { sequence } from "astro/middleware";

export const onRequest = sequence((ctx, next) => {
	if (ctx.url.pathname == "/oldlink") {
		return next("/newlink");
	}
	return next();
});

// src/pages/newlink.ts
import type { APIRoute } from "astro";

export const POST: APIRoute = async ({ request }) => {
	const data = await request.json();
	return new Response(JSON.stringify(data), { status: 200 });
};

// package.json
///...
"scripts": {
    "devnode": "astro dev"
    "devbun": "bunx --bun astro dev"
}
//...

When I run this setup with Node (npm run devnode or bun run devnode), it works. But with Bun (bun run devbun), I get [ERROR] Body already used.

My minimal reproducible example (which runs on Node) still stands though. So maybe this is a broader issue?

[xriter]

Update:
For now I have been able to fix my particular use-case by replacing return next('/form') with return context.rewrite('/form').

I do still think this issue is worth investigating further.

Looking at these docs: https://docs.astro.build/en/guides/middleware/#rewriting
...the third example under Rewriting where they showcase using something like return next('/') to rewrite a route.
I think the code as in my minimal reproducible example should still work...