Better errors for when response is already sent (#6719)

* Better errors for when response is already sent

* Changeset

* Casing

* Change to a 300x error

.changeset/red-parents-knock.md

@@ -0,0 +1,10 @@
+---
+'astro': patch
+---
+
+Better errors for when response is already sent
+
+This adds clearer error messaging when a Response has already been sent to the browser and the developer attempts to use:
+
+- Astro.cookies.set
+- Astro.redirect

packages/astro/src/core/app/index.ts

@@ -29,6 +29,7 @@ export { deserializeManifest } from './common.js';
 
 export const pagesVirtualModuleId = '@astrojs-pages-virtual-entry';
 export const resolvedPagesVirtualModuleId = '\0' + pagesVirtualModuleId;
+const responseSentSymbol = Symbol.for('astro.responseSent');
 
 export interface MatchOptions {
 	matchNotFound?: boolean | undefined;
@@ -201,6 +202,7 @@ export class App {
 			});
 
 			const response = await renderPage(mod, ctx, this.#env);
+			Reflect.set(request, responseSentSymbol, true)
 			return response;
 		} catch (err: any) {
 			error(this.#logging, 'ssr', err.stack || err.message || String(err));

packages/astro/src/core/cookies/cookies.ts

@@ -1,5 +1,6 @@
 import type { CookieSerializeOptions } from 'cookie';
 import { parse, serialize } from 'cookie';
+import { AstroError, AstroErrorData } from '../errors/index.js';
 
 interface AstroCookieSetOptions {
 	domain?: string;
@@ -33,6 +34,7 @@ interface AstroCookiesInterface {
 
 const DELETED_EXPIRATION = new Date(0);
 const DELETED_VALUE = 'deleted';
+const responseSentSymbol = Symbol.for('astro.responseSent');
 
 class AstroCookie implements AstroCookieInterface {
 	constructor(public value: string | undefined) {}
@@ -160,6 +162,12 @@ class AstroCookies implements AstroCookiesInterface {
 			serialize(key, serializedValue, serializeOptions),
 			true,
 		]);
+
+		if((this.#request as any)[responseSentSymbol]) {
+			throw new AstroError({
+				...AstroErrorData.ResponseSentError,
+			});
+		}
 	}
 
 	/**

packages/astro/src/core/errors/errors-data.ts

@@ -618,6 +618,16 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
 			}`,
 		hint: 'This is often caused by a typo in the image path. Please make sure the file exists, and is spelled correctly.',
 	},
+	/**
+	 * @docs
+	 * @description
+	 * Making changes to the response, such as setting headers, cookies, and the status code cannot be done outside of page components.
+	 */
+	ResponseSentError: {
+		title: 'Unable to set response',
+		code: 3030,
+		message: 'The response has already been sent to the browser and cannot be altered.',
+	},
 	// No headings here, that way Vite errors are merged with Astro ones in the docs, which makes more sense to users.
 	// Vite Errors - 4xxx
 	/**

packages/astro/src/core/render/result.ts

@@ -16,6 +16,7 @@ import { AstroError, AstroErrorData } from '../errors/index.js';
 import { warn, type LogOptions } from '../logger/core.js';
 
 const clientAddressSymbol = Symbol.for('astro.clientAddress');
+const responseSentSymbol = Symbol.for('astro.responseSent');
 
 function onlyAvailableInSSR(name: 'Astro.redirect') {
 	return function _onlyAvailableInSSR() {
@@ -197,6 +198,13 @@ export function createResult(args: CreateResultArgs): SSRResult {
 				url,
 				redirect: args.ssr
 					? (path, status) => {
+							// If the response is already sent, error as we cannot proceed with the redirect.
+							if((request as any)[responseSentSymbol]) {
+								throw new AstroError({
+									...AstroErrorData.ResponseSentError,
+								});
+							}
+
 							return new Response(null, {
 								status: status || 302,
 								headers: {

packages/astro/src/runtime/server/render/common.ts

@@ -20,7 +20,7 @@ export const decoder = new TextDecoder();
 // Rendering produces either marked strings of HTML or instructions for hydration.
 // These directive instructions bubble all the way up to renderPage so that we
 // can ensure they are added only once, and as soon as possible.
-export function stringifyChunk(result: SSRResult, chunk: string | SlotString | RenderInstruction) {
+export function stringifyChunk(result: SSRResult, chunk: string | SlotString | RenderInstruction): string {
 	if (typeof (chunk as any).type === 'string') {
 		const instruction = chunk as RenderInstruction;
 		switch (instruction.type) {

packages/astro/src/vite-plugin-astro-server/response.ts

@@ -99,6 +99,7 @@ export async function writeWebResponse(res: http.ServerResponse, webResponse: Re
 	res.end();
 }
 
-export async function writeSSRResult(webResponse: Response, res: http.ServerResponse) {
+export async function writeSSRResult(webRequest: Request, webResponse: Response, res: http.ServerResponse) {
+	Reflect.set(webRequest, Symbol.for('astro.responseSent'), true);
 	return writeWebResponse(res, webResponse);
 }

packages/astro/src/vite-plugin-astro-server/route.ts

@@ -216,6 +216,6 @@ export async function handleRoute(
 	} else {
 		const result = await renderPage(options);
 		throwIfRedirectNotAllowed(result, config);
-		return await writeSSRResult(result, res);
+		return await writeSSRResult(request, result, res);
 	}
 }

packages/astro/test/fixtures/ssr-redirect/src/components/redirect.astro

@@ -0,0 +1,3 @@
+---
+return Astro.redirect('/login');
+---

packages/astro/test/fixtures/ssr-redirect/src/pages/late.astro

@@ -0,0 +1,12 @@
+---
+import Redirect from '../components/redirect.astro';
+---
+<html>
+	<head>
+		<title>Testing</title>
+	</head>
+	<body>
+		<h1>Testing</h1>
+		<Redirect />
+	</body>
+</html>

packages/astro/test/ssr-redirect.test.js

@@ -22,4 +22,16 @@ describe('Astro.redirect', () => {
 		expect(response.status).to.equal(302);
 		expect(response.headers.get('location')).to.equal('/login');
 	});
+
+	it('Warns when used inside a component', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/late');
+		const response = await app.render(request);
+		try {
+			const text = await response.text();
+			expect(false).to.equal(true);
+		} catch(e) {
+			expect(e.message).to.equal('The response has already been sent to the browser and cannot be altered.');
+		}
+	});
 });

packages/astro/test/units/cookies/error.test.js

@@ -0,0 +1,21 @@
+import { expect } from 'chai';
+import { AstroCookies } from '../../../dist/core/cookies/index.js';
+import { apply as applyPolyfill } from '../../../dist/core/polyfill.js';
+
+applyPolyfill();
+
+describe('astro/src/core/cookies', () => {
+	describe('errors', () => {
+		it('Produces an error if the response is already sent', () => {
+			const req = new Request('http://example.com/', {});
+			const cookies = new AstroCookies(req);
+			req[Symbol.for('astro.responseSent')] = true;
+			try {
+				cookies.set('foo', 'bar');
+				expect(false).to.equal(true);
+			} catch(err) {
+				expect(err.errorCode).to.equal(3030);
+			}
+		});
+	});
+});