diff --git a/.changeset/red-parents-knock.md b/.changeset/red-parents-knock.md new file mode 100644 index 000000000000..b169a31931af --- /dev/null +++ b/.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 diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index e43e9a8ff73c..2088baca4aa2 100644 --- a/packages/astro/src/core/app/index.ts +++ b/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)); diff --git a/packages/astro/src/core/cookies/cookies.ts b/packages/astro/src/core/cookies/cookies.ts index 73996f51fb42..c9429d142afa 100644 --- a/packages/astro/src/core/cookies/cookies.ts +++ b/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, + }); + } } /** diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 61a987638e9c..c5a3b7fad919 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/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 /** diff --git a/packages/astro/src/core/render/result.ts b/packages/astro/src/core/render/result.ts index 5305400cbe5b..c5f6a3bc9693 100644 --- a/packages/astro/src/core/render/result.ts +++ b/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: { diff --git a/packages/astro/src/runtime/server/render/common.ts b/packages/astro/src/runtime/server/render/common.ts index 6892a0f34515..53eb3de189a7 100644 --- a/packages/astro/src/runtime/server/render/common.ts +++ b/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) { diff --git a/packages/astro/src/vite-plugin-astro-server/response.ts b/packages/astro/src/vite-plugin-astro-server/response.ts index a7cc6e093609..7aef08a4ca83 100644 --- a/packages/astro/src/vite-plugin-astro-server/response.ts +++ b/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); } diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index de58daa71316..79232f10fc69 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/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); } } diff --git a/packages/astro/test/fixtures/ssr-redirect/src/components/redirect.astro b/packages/astro/test/fixtures/ssr-redirect/src/components/redirect.astro new file mode 100644 index 000000000000..f8fc808e7ade --- /dev/null +++ b/packages/astro/test/fixtures/ssr-redirect/src/components/redirect.astro @@ -0,0 +1,3 @@ +--- +return Astro.redirect('/login'); +--- diff --git a/packages/astro/test/fixtures/ssr-redirect/src/pages/late.astro b/packages/astro/test/fixtures/ssr-redirect/src/pages/late.astro new file mode 100644 index 000000000000..dcfedb8da6a1 --- /dev/null +++ b/packages/astro/test/fixtures/ssr-redirect/src/pages/late.astro @@ -0,0 +1,12 @@ +--- +import Redirect from '../components/redirect.astro'; +--- + + + Testing + + +

Testing

+ + + diff --git a/packages/astro/test/ssr-redirect.test.js b/packages/astro/test/ssr-redirect.test.js index dd23e26d382f..3374b3246816 100644 --- a/packages/astro/test/ssr-redirect.test.js +++ b/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.'); + } + }); }); diff --git a/packages/astro/test/units/cookies/error.test.js b/packages/astro/test/units/cookies/error.test.js new file mode 100644 index 000000000000..ecea13b71db7 --- /dev/null +++ b/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); + } + }); + }); +});