diff --git a/.changeset/sour-seas-hang.md b/.changeset/sour-seas-hang.md new file mode 100644 index 0000000000..20b98db303 --- /dev/null +++ b/.changeset/sour-seas-hang.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a regression where a response created with `Response.redirect` or containing `null` as the body never completed in node-based adapters. diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index db61157e31..ccb2068306 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -98,7 +98,7 @@ export class NodeApp extends App { static async writeResponse(source: Response, destination: ServerResponse) { const { status, headers, body } = source; destination.writeHead(status, createOutgoingHttpHeaders(headers)); - if (body) { + if (!body) return destination.end(); try { const reader = body.getReader(); destination.on('close', () => { @@ -123,7 +123,6 @@ export class NodeApp extends App { } catch { destination.end('Internal server error'); } - } } } diff --git a/packages/integrations/markdoc/package.json b/packages/integrations/markdoc/package.json index 7af2e1b154..3b84622993 100644 --- a/packages/integrations/markdoc/package.json +++ b/packages/integrations/markdoc/package.json @@ -59,7 +59,7 @@ "build": "astro-scripts build \"src/**/*.ts\" && tsc", "build:ci": "astro-scripts build \"src/**/*.ts\"", "dev": "astro-scripts dev \"src/**/*.ts\"", - "test": "astro-scripts test \"test/**/*.test.js\"" + "test": "astro-scripts test --timeout 40000 \"test/**/*.test.js\"" }, "dependencies": { "@astrojs/internal-helpers": "workspace:*", diff --git a/packages/integrations/node/test/api-route.test.js b/packages/integrations/node/test/api-route.test.js index 80e99a2ccd..9e0165cb98 100644 --- a/packages/integrations/node/test/api-route.test.js +++ b/packages/integrations/node/test/api-route.test.js @@ -1,12 +1,16 @@ import nodejs from '../dist/index.js'; import { loadFixture, createRequestAndResponse } from './test-utils.js'; import crypto from 'node:crypto'; -import { describe, it, before } from 'node:test'; +import { describe, it, before, after } from 'node:test'; import * as assert from 'node:assert/strict'; describe('API routes', () => { /** @type {import('./test-utils').Fixture} */ let fixture; + /** @type {import('astro/src/@types/astro.js').PreviewServer} */ + let previewServer; + /** @type {URL} */ + let baseUri; before(async () => { fixture = await loadFixture({ @@ -15,8 +19,12 @@ describe('API routes', () => { adapter: nodejs({ mode: 'middleware' }), }); await fixture.build(); + previewServer = await fixture.preview(); + baseUri = new URL(`http://${previewServer.host ?? 'localhost'}:${previewServer.port}/`); }); + after(() => previewServer.stop()); + it('Can get the request body', async () => { const { handler } = await import('./fixtures/api-route/dist/server/entry.mjs'); let { req, res, done } = createRequestAndResponse({ @@ -109,4 +117,37 @@ describe('API routes', () => { assert.deepEqual(locals, { cancelledByTheServer: true }); }); + + it('Can respond with SSR redirect', async () => { + const controller = new AbortController(); + setTimeout(() => controller.abort(), 1000); + const response = await fetch(new URL('/redirect', baseUri), { + redirect: 'manual', + signal: controller.signal, + }); + assert.equal(response.status, 302); + assert.equal(response.headers.get('location'), '/destination'); + }); + + it('Can respond with Astro.redirect', async () => { + const controller = new AbortController(); + setTimeout(() => controller.abort(), 1000); + const response = await fetch(new URL('/astro-redirect', baseUri), { + redirect: 'manual', + signal: controller.signal, + }); + assert.equal(response.status, 303); + assert.equal(response.headers.get('location'), '/destination'); + }); + + it('Can respond with Response.redirect', async () => { + const controller = new AbortController(); + setTimeout(() => controller.abort(), 1000); + const response = await fetch(new URL('/response-redirect', baseUri), { + redirect: 'manual', + signal: controller.signal, + }); + assert.equal(response.status, 307); + assert.equal(response.headers.get('location'), String(new URL('/destination', baseUri))); + }); }); diff --git a/packages/integrations/node/test/errors.test.js b/packages/integrations/node/test/errors.test.js index 76bff13265..8d54bcd4ed 100644 --- a/packages/integrations/node/test/errors.test.js +++ b/packages/integrations/node/test/errors.test.js @@ -32,16 +32,25 @@ describe('Errors', () => { }); it('generator that throws called in template', async () => { + const result = ['

Astro

1', 'Internal server error']; + /** @type {Response} */ const res = await fixture.fetch('/generator'); const reader = res.body.getReader(); const decoder = new TextDecoder(); - const expect = async ({ done, value }) => { - const result = await reader.read(); - assert.equal(result.done, done); - if (!done) assert.equal(decoder.decode(result.value), value); - }; - await expect({ done: false, value: '

Astro

1Internal server error' }); - await expect({ done: true }); + const chunk1 = await reader.read(); + const chunk2 = await reader.read(); + const chunk3 = await reader.read(); + assert.equal(chunk1.done, false); + if (chunk2.done) { + assert.equal(decoder.decode(chunk1.value), result.join("")); + } + else if (chunk3.done) { + assert.equal(decoder.decode(chunk1.value), result[0]); + assert.equal(decoder.decode(chunk2.value), result[1]); + } + else { + throw new Error('The response should take at most 2 chunks.'); + } }); }); diff --git a/packages/integrations/node/test/fixtures/api-route/src/pages/astro-redirect.astro b/packages/integrations/node/test/fixtures/api-route/src/pages/astro-redirect.astro new file mode 100644 index 0000000000..65a8765e81 --- /dev/null +++ b/packages/integrations/node/test/fixtures/api-route/src/pages/astro-redirect.astro @@ -0,0 +1,3 @@ +--- +return Astro.redirect('/destination', 303); +--- diff --git a/packages/integrations/node/test/fixtures/api-route/src/pages/redirect.ts b/packages/integrations/node/test/fixtures/api-route/src/pages/redirect.ts new file mode 100644 index 0000000000..baf22c93ec --- /dev/null +++ b/packages/integrations/node/test/fixtures/api-route/src/pages/redirect.ts @@ -0,0 +1,5 @@ +import { APIContext } from 'astro'; + +export async function GET({ redirect }: APIContext) { + return redirect('/destination'); +} diff --git a/packages/integrations/node/test/fixtures/api-route/src/pages/response-redirect.ts b/packages/integrations/node/test/fixtures/api-route/src/pages/response-redirect.ts new file mode 100644 index 0000000000..1dfa8bb3c6 --- /dev/null +++ b/packages/integrations/node/test/fixtures/api-route/src/pages/response-redirect.ts @@ -0,0 +1,5 @@ +import { APIContext } from 'astro'; + +export async function GET({ url: requestUrl }: APIContext) { + return Response.redirect(new URL('/destination', requestUrl), 307); +} diff --git a/packages/integrations/node/test/prerender.test.js b/packages/integrations/node/test/prerender.test.js index 597794be06..7036c913c4 100644 --- a/packages/integrations/node/test/prerender.test.js +++ b/packages/integrations/node/test/prerender.test.js @@ -82,8 +82,7 @@ describe('Prerendering', () => { }); const html = await res.text(); const $ = cheerio.load(html); - assert.equal(res.status, 301); - assert.equal(res.headers.get('location'), '/some-base/two/'); + assert.equal(res.status, 200); assert.equal($('h1').text(), 'Two'); }); }); @@ -171,8 +170,8 @@ describe('Prerendering', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route', async () => { @@ -180,8 +179,8 @@ describe('Prerendering', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Two'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Two'); }); }); }); @@ -252,8 +251,9 @@ describe('Hybrid rendering', () => { const res = await fetch(`http://${server.host}:${server.port}/some-base/one`, { redirect: 'manual', }); - assert.equal(res.status, 301); - assert.equal(res.headers.get('location'), '/some-base/one/'); + const html = await res.text(); + const $ = cheerio.load(html); + assert.equal(res.status, 200); assert.equal($('h1').text(), 'One'); }); }); @@ -342,8 +342,8 @@ describe('Hybrid rendering', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('shared'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'shared'); }); }); });