From 4d23caed76081742ddffe99841f5a1cb672f8d99 Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Thu, 1 Feb 2024 21:24:21 +0000 Subject: [PATCH] fix(NodeApp): fix responses with null bodies never completing (#9931) * fix(NodeApp): fix responses with null bodies never completing * add changeset * add test * chore(tests): restore correct assertions * adjust incorrect test * added Astro.redirect and Response.redirect test cases * updated incorrect HTTP status * adjust api-routes.test.js after cherry-pick * bup markdoc test timeout --------- Co-authored-by: Emanuele Stoppa Co-authored-by: Friedemann Sommer --- .../integrations/node/test/api-route.test.js | 43 ++++++++++++++++++- .../integrations/node/test/errors.test.js | 23 +++++++--- .../api-route/src/pages/astro-redirect.astro | 3 ++ .../fixtures/api-route/src/pages/redirect.ts | 5 +++ .../api-route/src/pages/response-redirect.ts | 5 +++ .../integrations/node/test/prerender.test.js | 20 ++++----- 6 files changed, 81 insertions(+), 18 deletions(-) create mode 100644 packages/integrations/node/test/fixtures/api-route/src/pages/astro-redirect.astro create mode 100644 packages/integrations/node/test/fixtures/api-route/src/pages/redirect.ts create mode 100644 packages/integrations/node/test/fixtures/api-route/src/pages/response-redirect.ts 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'); }); }); });