summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--.changeset/red-parents-knock.md10
-rw-r--r--packages/astro/src/core/app/index.ts2
-rw-r--r--packages/astro/src/core/cookies/cookies.ts8
-rw-r--r--packages/astro/src/core/errors/errors-data.ts10
-rw-r--r--packages/astro/src/core/render/result.ts8
-rw-r--r--packages/astro/src/runtime/server/render/common.ts2
-rw-r--r--packages/astro/src/vite-plugin-astro-server/response.ts3
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts2
-rw-r--r--packages/astro/test/fixtures/ssr-redirect/src/components/redirect.astro3
-rw-r--r--packages/astro/test/fixtures/ssr-redirect/src/pages/late.astro12
-rw-r--r--packages/astro/test/ssr-redirect.test.js12
-rw-r--r--packages/astro/test/units/cookies/error.test.js21
12 files changed, 90 insertions, 3 deletions
diff --git a/.changeset/red-parents-knock.md b/.changeset/red-parents-knock.md
new file mode 100644
index 000000000..b169a3193
--- /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 e43e9a8ff..2088baca4 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 73996f51f..c9429d142 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 61a987638..c5a3b7fad 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 5305400cb..c5f6a3bc9 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 6892a0f34..53eb3de18 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 a7cc6e093..7aef08a4c 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 de58daa71..79232f10f 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 000000000..f8fc808e7
--- /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 000000000..dcfedb8da
--- /dev/null
+++ b/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>
diff --git a/packages/astro/test/ssr-redirect.test.js b/packages/astro/test/ssr-redirect.test.js
index dd23e26d3..3374b3246 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 000000000..ecea13b71
--- /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);
+ }
+ });
+ });
+});